netdev - Re: [PATCH 2/2] IB/ipoib: fix GRO merge failure for IPoIB originated TCP streams

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL1RGDUm8ROxFFMa+D1ZD5jF+cK+kV8aEVzspgnZFNXeuai+fA@mail.gmail.com>
Date:	Sun, 29 Jan 2012 20:36:13 -0800
From:	Roland Dreier <roland@...nel.org>
To:	Or Gerlitz <ogerlitz@...lanox.com>
Cc:	linux-rdma <linux-rdma@...r.kernel.org>,
	Shlomo Pongratz <shlomop@...lanox.com>, netdev@...r.kernel.org
Subject: Re: [PATCH 2/2] IB/ipoib: fix GRO merge failure for IPoIB originated
 TCP streams

On Thu, Jan 26, 2012 at 6:43 AM, Or Gerlitz <ogerlitz@...lanox.com> wrote:
> From: Shlomo Pongratz <shlomop@...lanox.com>
>
> 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.

> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
> index 4115be5..89cfaf7 100644
> --- 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?

 - R.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html