[ovs-dev] [PATCH] [patch v2] datapath: Treat GSO skbs as if they were fragments

Simon Horman horms at verge.net.au
Wed Jan 5 00:55:46 UTC 2011


On Tue, Jan 04, 2011 at 05:52:24PM -0500, Jesse Gross wrote:
> On Sun, Jan 2, 2011 at 7:11 PM, Simon Horman <horms at verge.net.au> wrote:
> > In dp_output_control() UDP GSO skbs are split into fragments which are
> > passed to userspace.  So the resulting flow set-up by the controller
> > (I am using ovs-vswitchd) is created based on a fragment.  This means
> > that the UDP source and destination port of the flow is zero.
> >
> > In order for the datapath to match the resulting flow flow_extract() needs
> > to treat UDP GSO skbs as if they are fragments.  That is, set the UDP
> > source and destination port to 0.
> >
> > A flow established for a UDP GSO skb with this change won't match any
> > subsequent non-GSO skbs, they will need to be passed to the controller and
> > a new flow established. But without this change no UDP GSO skbs will ever
> > match any flow.
> >
> > I noticed this while using KVM using virtio with VhostNet and netperf's
> > UDP_STREAM test. The result was that the test sent ~5Gbit/s but only a
> > small fraction of that was received by the other side. Much less than the
> > 1Gbit/s available on the physical link between the host (and guest) and the
> > machine running netserver. 100% of one of the host's CPUs was consumed, 50%
> > for the host and 50% for the guest.  The host consumption was contributed
> > to largely by ovs-vswitchd.
> >
> > With this change I get a much nicer result of a fraction under 1Gbit/s sent
> > and almost all packets ending up at the other end.
> >
> > Signed-off-by: Simon Horman <horms at verge.net.au>
> >
> > ---
> >
> > v2
> > * Only handle UDP GSO skbs rather than all GSO skbs
> >  as suggested by Jesse Gross
> > ---
> >  datapath/flow.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> >
> > diff --git a/datapath/flow.c b/datapath/flow.c
> > index b33b315..36c2374 100644
> > --- a/datapath/flow.c
> > +++ b/datapath/flow.c
> > @@ -333,7 +333,8 @@ int flow_extract(struct sk_buff *skb, u16 in_port, struct odp_flow_key *key,
> >                key->nw_proto = nh->protocol;
> >
> >                /* Transport layer. */
> > -               if (!(nh->frag_off & htons(IP_MF | IP_OFFSET))) {
> > +               if (!(nh->frag_off & htons(IP_MF | IP_OFFSET)) &&
> > +                   !(key->nw_proto == IPPROTO_UDP && skb_is_gso(skb))) {
> 
> This looks right but what if we just did !(skb_shinfo(skb)->gso_type &
> SKB_GSO_UDP)?  That will trade two hard to predict branches for one
> easy one in the common case.

Yes, I think that is a good idea.
I'll run it through my test and get back to you.





More information about the dev mailing list