[ovs-dev] [PATCH 2/2] datapath: Disable LRO from userspace instead of the kernel.
jesse at nicira.com
Sat Aug 27 06:59:02 PDT 2011
On Sat, Aug 27, 2011 at 3:56 PM, Justin Pettit <jpettit at nicira.com> wrote:
> On Aug 19, 2011, at 8:39 PM, Jesse Gross wrote:
>> The final piece that I just thought of, is the relationship of the MTU
>> check in vport_send() and skb_warn_if_lro(). The former will catch
>> hardware LRO and interfaces with mismatched MTU and the later the
>> deprecated (but still extant software LRO, precursor to GRO). When
>> disabling LRO, we will turn off both the software and hardware
>> versions so there is some overlap. The checks are useful but they'll
>> trigger warnings if we start using LRO. That's not strictly a
>> backwards compatibility problem but it will still create a little bit
>> of a versioning mess. What if we just move all of them to
>> netdev_send(), which is where they are actually useful?
> If I understand what you're proposing, I'm not entirely clear on the benefit. What's the benefit of putting those checks in netdev_send() as opposed to vport_send() and netdev_port_receive()? I may be misunderstanding what you're suggesting. Were you thinking something along the lines of the attached patch?
What I'm thinking about is what happens when we actually implement
this optimization, assuming that the userspace/kernel interface is
locked down. When userspace stops disabling LRO, these checks will
start triggering and will drop packets. To support this we'll have to
remove or relax those checks, which is fine because we're just
implementing a feature that was previously unsupported. However, in
order to use this feature userspace will need to detect whether it
exists, which in this case means some kind of kernel version check
because there's nothing directly visible from userspace.
To avoid the above mess, I thought that maybe we should just implement
the kernel portion of this since it's fairly simple and then the
problem just goes away. The easiest way to do that is just drop the
checks but they actually are somewhat useful for debugging
configuration problems. The MTU check is really only useful in the
netdev case because that's where we hit real devices with MTUs and is
the place where LRO is actually a problem. The optimization that I
have in mind is useful for internal devices (which have an MTU but
don't care) and it's already a no-op for OVS virtual devices like
tunnels and patch ports because they don't have MTUs at all. That's
why I think that pushing the check into the netdev vport code only
solves the problem.
The patch is basically what I had in mind, though you can now remove
the test for no MTU and I would also drop the packet and record the
error if skb_warn_if_lro() triggers (it returns a bool).
More information about the dev