From: Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Or Gerlitz <ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Cc: Roland Dreier <roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
Herbert Xu
<herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>,
davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
linux-rdma <linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
Shlomo Pongratz <shlomop-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH 2/2] IB/ipoib: fix GRO merge failure for IPoIB originated TCP streams
Date: Mon, 30 Jan 2012 09:04:32 +0100 [thread overview]
Message-ID: <1327910672.2891.12.camel@edumazet-laptop> (raw)
In-Reply-To: <4F264A6C.3070706-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
Le lundi 30 janvier 2012 à 09:44 +0200, Or Gerlitz a écrit :
> On 1/30/2012 6:36 AM, Roland Dreier wrote:
> > On Thu, Jan 26, 2012 at 6:43 AM, Or Gerlitz<ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org> wrote:
> >> The GRO flow makes a check in every layer to ensure the packets
> >> are actually merged only if they match at all layers.
> >>
> >> The first GRO check, at L2 always fails for IPoIB, since it assumes
> >> that all packets have 14 bytes of Ethernet link layer header. Using the
> >> IPoIB header will not help here either, since its only four bytes. To
> >> overcome this, the skb mac header pointer is set to an area within the
> >> packet IB GRH headroom, such that later, the L2 check done by GRO
> >> succeeds and it can move to checks at the network and transport layers.
> >
> >> --- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> >> +++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> >> @@ -286,10 +287,20 @@ static void ipoib_ib_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
> >> else
> >> skb->pkt_type = PACKET_MULTICAST;
> >>
> >> - skb_pull(skb, IB_GRH_BYTES);
> >> + /*
> >> + * GRO first does L2 compares (14 bytes). We must not let it start from
> >> + * the IPoIB header as ten octets of the IP header, containing fields
> >> + * which vary from packet to packet will cause non-merging of packets.
> >> + * from the same TCP stream.
> >> + */
> >> + psgid = skb_pull(skb, offsetof(struct ib_grh, sgid));
> >> + /* if there's no GRH, that area could contain random data */
> >> + if (!(wc->wc_flags& IB_WC_GRH))
> >> + memset(psgid, 0, 16);
> >> + skb_reset_mac_header(skb);
> >> + skb_pull(skb, IB_GRH_BYTES - offsetof(struct ib_grh, sgid));
> >>
> >> skb->protocol = ((struct ipoib_header *) skb->data)->proto;
> >> - skb_reset_mac_header(skb);
> >
> > This seems like a really weird place to fix this. Wouldn't it
> > make more sense to fix the GRO check to handle non-ethernet L2 headers?
>
> Yes, we can do that as well. Herbert, Dave, would it be enough here, to
> skip the Ethernet header and vlan comparison for skbs whose associated
> netdevice type isn't ARPHRD_ETHER? e.g something along the lines of:
>
> > diff --git a/net/core/dev.c b/net/core/dev.c
> > index 115dee1..c529f5a 100644
> > --- a/net/core/dev.c
> > +++ b/net/core/dev.c
> > @@ -3505,9 +3505,11 @@ __napi_gro_receive(struct napi_struct *napi,
> > struct sk_buff *skb)
> > unsigned long diffs;
> >
> > diffs = (unsigned long)p->dev ^ (unsigned long)skb->dev;
> > - diffs |= p->vlan_tci ^ skb->vlan_tci;
> > - diffs |= compare_ether_header(skb_mac_header(p),
> > - skb_gro_mac_header(skb));
> > + if (!diffs && p->dev->type == ARPHRD_ETHER) {
> > + diffs |= p->vlan_tci ^ skb->vlan_tci;
> > + diffs |= compare_ether_header(skb_mac_header(p),
> > +
> > skb_gro_mac_header(skb));
> > + }
> > NAPI_GRO_CB(p)->same_flow = !diffs;
> > NAPI_GRO_CB(p)->flush = 0;
Hmm, do we really need to compare ether header, thats the question.
IMHO, GRO could avoid this check, as legal trafic could be never merged
(eg multipath)
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2012-01-30 8:04 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <alpine.LRH.2.00.1201261624540.30384@ogerlitz.voltaire.com>
[not found] ` <alpine.LRH.2.00.1201261642340.31408@ogerlitz.voltaire.com>
[not found] ` <alpine.LRH.2.00.1201261642340.31408-VYr5/9ddeaGSIdy2EShu12Xnswh1EIUO@public.gmane.org>
2012-01-30 4:36 ` [PATCH 2/2] IB/ipoib: fix GRO merge failure for IPoIB originated TCP streams Roland Dreier
[not found] ` <CAL1RGDUm8ROxFFMa+D1ZD5jF+cK+kV8aEVzspgnZFNXeuai+fA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-01-30 7:44 ` Shlomo Pongratz
[not found] ` <36F7E4A28C18BE4DB7C86058E7B607240BE9687A-SlGPd/IId7auSA5JZHE7gA@public.gmane.org>
2012-01-30 18:11 ` Roland Dreier
[not found] ` <CAL1RGDXjjQ-PhCv-9WJX45NuovC9XiS=_7507OsyvLW_gBaJ5g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-01-30 18:16 ` Or Gerlitz
[not found] ` <CAJZOPZLhgDysSASyMLNpOrmGzEyfyHAQjGLVei6ZNSFfb7TM1w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-01-30 19:00 ` Roland Dreier
2012-01-30 7:44 ` Or Gerlitz
[not found] ` <4F264A6C.3070706-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-01-30 8:04 ` Eric Dumazet [this message]
2012-01-30 8:11 ` Or Gerlitz
2012-01-30 8:18 ` Herbert Xu
[not found] ` <20120130081849.GA7848-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
2012-01-30 8:53 ` Eric Dumazet
2012-01-30 8:57 ` Herbert Xu
[not found] ` <20120130085742.GA8262-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
2012-01-30 16:43 ` David Miller
2012-02-01 8:23 ` Or Gerlitz
[not found] ` <CAJZOPZJb1HvcS0XXKLvoDuoi1EfYTY-awwY2g0aHWoS=4qmdyQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-01 8:38 ` Herbert Xu
[not found] ` <20120201083837.GA7081-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org>
2012-02-01 9:43 ` Or Gerlitz
2012-02-01 14:07 ` Eric Dumazet
2012-02-02 14:01 ` Or Gerlitz
[not found] ` <4F2A974B.209-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-02 14:38 ` Eric Dumazet
2012-02-02 14:44 ` Eric Dumazet
2012-02-02 21:42 ` Or Gerlitz
2012-02-02 15:43 ` Or Gerlitz
2012-01-30 8:25 ` Michał Mirosław
2012-02-02 21:58 ` Or Gerlitz
[not found] ` <alpine.LRH.2.00.1202022352560.30300-VYr5/9ddeaGSIdy2EShu12Xnswh1EIUO@public.gmane.org>
2012-02-03 7:18 ` Hefty, Sean
[not found] ` <1828884A29C6694DAF28B7E6B8A823733349A5D2-P5GAC/sN6hmkrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2012-02-03 9:00 ` Eric Dumazet
2012-02-03 20:24 ` Hefty, Sean
[not found] ` <1828884A29C6694DAF28B7E6B8A823733349A6C9-P5GAC/sN6hmkrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2012-02-06 15:05 ` Or Gerlitz
[not found] ` <4F2FEC36.6090800-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-06 15:21 ` Eric Dumazet
2012-02-06 15:22 ` Or Gerlitz
[not found] ` <4F2FF050.7040400-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-06 16:27 ` [PATCH net-next] gro: introduce gro_mac_header_len Eric Dumazet
2012-02-06 16:31 ` David Miller
[not found] ` <20120206.113145.1284864994961472499.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 16:44 ` Or Gerlitz
[not found] ` <4F300353.2080705-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-06 17:00 ` David Miller
2012-02-06 16:47 ` [PATCH net-next V2] " Eric Dumazet
2012-02-06 16:58 ` David Miller
2012-02-06 17:07 ` Eric Dumazet
2012-02-06 17:11 ` Or Gerlitz
[not found] ` <4F3009AE.2090605-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-06 17:19 ` Eric Dumazet
[not found] ` <20120206.115859.1384761795375582044.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 17:09 ` Or Gerlitz
2012-02-06 17:23 ` Roland Dreier
[not found] ` <CAG4TOxOBCbEEOtP62ZM1R3-6umebdbJFkK0bnjphLCZOgHzt1g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-06 19:12 ` David Miller
[not found] ` <20120206.141223.332863167187002998.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 19:23 ` Roland Dreier
[not found] ` <CAG4TOxNgJ3=AFuA==Km815YzW8eQ7nD_UAzkXLSXcAyaCAAvPg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-06 19:32 ` David Miller
[not found] ` <20120206.143230.1415707004934341114.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 19:51 ` David Miller
[not found] ` <20120206.145148.558736903670696169.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 19:56 ` David Miller
[not found] ` <20120206.145652.1575591691467905094.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-06 20:01 ` Roland Dreier
[not found] ` <CAG4TOxNcB8+SP3f0WhZG1GZ481s+=7pVRv91vzAbwDzUZD8g_A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-06 20:15 ` David Miller
[not found] ` <20120206.151509.1959432192519622134.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-07 19:51 ` Roland Dreier
2012-02-07 20:33 ` David Miller
[not found] ` <20120207.153325.1941809701255235550.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-07 20:34 ` Roland Dreier
[not found] ` <CAG4TOxP2iEgT64L8vAR6P139U-HyHSxU8gdWw+cLsDwWSGkrwA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-08 0:51 ` [PATCH] IPoIB: Stop lying about hard_header_len and use skb->cb to stash LL addresses Roland Dreier
2012-02-08 7:29 ` Hefty, Sean
[not found] ` <1828884A29C6694DAF28B7E6B8A82373374C1C26-P5GAC/sN6hmkrb+BlOpmy7fspsVTdybXVpNB7YpNyf8@public.gmane.org>
2012-02-08 7:50 ` Eric Dumazet
2012-02-08 14:28 ` Or Gerlitz
2012-02-08 20:50 ` David Miller
2012-02-06 19:59 ` [PATCH net-next V2] gro: introduce gro_mac_header_len Roland Dreier
2012-02-08 18:51 ` [PATCH net-next] gro: more generic L2 header check Eric Dumazet
2012-02-08 20:50 ` David Miller
[not found] ` <20120208.155027.599792363539233740.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-08 21:08 ` Or Gerlitz
[not found] ` <CAJZOPZL_zEbNwUfVOmQeODny7HDf24gOc1HStfBxim_UtD-kvA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-08 21:31 ` David Miller
[not found] ` <20120208.163159.2229331610142060560.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-08 21:49 ` Or Gerlitz
[not found] ` <CAJZOPZLLteDbm0prTN3-npubtFun=kO2DYT7ea=E-HyJ84gaiA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-08 23:09 ` David Miller
[not found] ` <20120208.180917.586628615268005115.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-08 23:20 ` Or Gerlitz
[not found] ` <CAJZOPZ+SyGsDpnK89wNWtBn4Jdzc2tg1diKjbHSv=n75XiG+ug-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-08 23:26 ` David Miller
[not found] ` <20120208.182629.1816927738607872730.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-09 10:46 ` Or Gerlitz
[not found] ` <4F33A3FC.1050205-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
2012-02-09 18:52 ` David Miller
[not found] ` <20120209.135235.957414875716615693.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-09 20:04 ` Or Gerlitz
2012-02-09 20:29 ` David Miller
[not found] ` <20120209.152943.1123862542883793404.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
2012-02-09 22:18 ` Or Gerlitz
[not found] ` <CAJZOPZKUG-tT+Zd=1EiLhgRxg4VEDrisBJQ99XtARMvK68UWfA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-02-09 22:28 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1327910672.2891.12.camel@edumazet-laptop \
--to=eric.dumazet-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
--cc=davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org \
--cc=herbert-lOAM2aK0SrRLBo1qDEOMRrpzq4S04n8Q@public.gmane.org \
--cc=linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
--cc=roland-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org \
--cc=shlomop-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).