On Fri, Dec 13, 2013 at 09:44:06AM +0900, Simon Horman wrote: > On Thu, Dec 12, 2013 at 09:56:18AM -0800, Ben Pfaff wrote: > > On Thu, Dec 12, 2013 at 05:38:59PM +0900, Simon Horman wrote: > > > If VLAN acceleration is used when the kernel receives a packet > > > then the outer-most VLAN tag will not be present in the packet > > > when it is received by netdev-linux. Rather, it will be present > > > in auxdata. > > > > > > This patch uses recvmsg() instead of recv() to read auxdata for > > > each packet and if the vlan_tid is set then it is added to the packet. > > > > In netdev_linux_rx_recv_sock(), I think we might want to > > ofpbuf_reserve() VLAN_HEADER_LEN bytes at the beginning, to ensure that > > there is headroom to insert a VLAN header. On the other hand, that > > would mean that we lose four bytes of tailroom that are important if the > > VLAN header is actually embedded in the packet. I think that means that > > we should advise callers to supply 4 bytes of space beyond what they > > think they need. > > I believe that sufficient headroom for one VLAN tag is already reserved > in the caller, dpif_netdev_run(): > > ofpbuf_reserve(&packet, DP_NETDEV_HEADROOM) > > Is that for some other purpose?
As-is, it's a layering violation to rely on that being there. If you want netdev_rx_recv() to rely on the caller reserving headroom for a VLAN headroom, then we need to document that. > > I don't think the cast here, or in netdev_linux_rx_recv(), is necessary: > > I believe the cast happens implicitly so I wanted to make > it more obvious what was going on. But I also believe that the > code will both compile and run fine without the cast. I will remove it > if you like. I prefer to remove it. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
