On Wed, Aug 18, 2010 at 12:46 AM, Simon Horman <span dir="ltr"><<a href="mailto:horms@verge.net.au">horms@verge.net.au</a>></span> wrote:<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im">
> > +/* XXX: Add IFF_OVS_DATAPATH to include/linux/if.h and use it<br>
> > + * in place of IFF_VPORT in this file. */<br>
> ><br>
><br>
> We could could do this with compatibility code: use the desired name in this<br>
> file and then define it to the appropriate value in<br>
> compat/include/linux/if.h. On pre-2.6.36 kernels it could be defined to 0,<br>
> defined to IFF_BRIDGE_PORT for later kernels, and then we wouldn't need to<br>
> define it all at once we make it upstream. This would remove a bunch of<br>
> #ifdef's from this file.<br>
<br>
</div>I think we can just unconditionally define it to IFF_BRIDGE_PORT,<br>
it will be unused with older kernels. It can then get a unique value<br>
after the merge.<br>
<br>
I don't think that this can remove any #ifdefs from the code<br>
as "netdev_vport->dev->priv_flags |= IFF_OVS_DATAPATH" and similar<br>
would still be incompatible with older kernels.<br></blockquote><div><br></div><div>If you #define it to be 0 on the older kernels it will be no-op and therefore not incompatible. That would then allow the removal of the #ifdef in the main code path.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="im"><br>
> There's also a block of code at the bottom of the file that is a hack to<br>
> enforce mutual exclusion with the bridge. Now that we can safely coexist<br>
> with the bridge, it can be #if'd out on appropriate kernels.<br>
<br>
</div>Done. Although registration of the datapath will fail if the bridge is<br>
present and vice versa because IFF_OVS_DATAPATH has the same value<br>
as IFF_BRIDGE_PORT (for now).<br>
</blockquote></div><br><div>Well you can only register one handler per device anyways, so it doesn't really matter that they have the same flag for now. The goal of the existing code block is to prevent both modules from loading at the same time (when there was only a single hook), since that will cause bad things to happen.</div>