On Thu, Dec 12, 2013 at 06:12:32PM -0800, Ben Pfaff wrote: > 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.
Understood. I'll implement your original suggestion. > > > 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. Will do. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
