netdev - Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Oct 2016 11:20:59 -0400
From:   Doug Ledford <dledford@...hat.com>
To:     David Miller <davem@...emloft.net>
Cc:     pabeni@...hat.com, linux-rdma@...r.kernel.org,
        sean.hefty@...el.com, hal.rosenstock@...il.com,
        netdev@...r.kernel.org
Subject: Re: [PATCH] IB/ipoib: move back the IB LL address into the hard
 header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header

On 10/13/2016 10:43 AM, David Miller wrote:
> From: Doug Ledford <dledford@...hat.com>
> Date: Thu, 13 Oct 2016 10:35:35 -0400
> 
>> On 10/13/2016 10:24 AM, David Miller wrote:
>>> From: Paolo Abeni <pabeni@...hat.com>
>>> Date: Tue, 11 Oct 2016 19:15:44 +0200
>>>
>>>> After the commit 9207f9d45b0a ("net: preserve IP control block
>>>> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
>>>> That destroy the IPoIB address information cached there,
>>>> causing a severe performance regression, as better described here:
>>>>
>>>> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
>>>>
>>>> This change moves the data cached by the IPoIB driver from the
>>>> skb control lock into the IPoIB hard header, as done before
>>>> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
>>>> and use skb->cb to stash LL addresses").
>>>> In order to avoid GRO issue, on packet reception, the IPoIB driver
>>>> stash into the skb a dummy pseudo header, so that the received
>>>> packets have actually a hard header matching the declared length.
>>>> Also the connected mode maximum mtu is reduced by 16 bytes to
>>>> cope with the increased hard header len.
>>>>
>>>> After this commit, IPoIB performances are back to pre-regression
>>>> value.
>>>>
>>>> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
>>>> Signed-off-by: Paolo Abeni <pabeni@...hat.com>
>>>
>>> Not providing an accurate hard_header_len causes many problems.
>>>
>>> In fact we recently fixed the mlxsw driver to stop doing this.
>>>
>>
>> Sure, but there are too many users of the cb struct, and whatever
>> problems you are saying there are by lying about the hard header len are
>> dwarfed by the problems caused by the inability to store the ll address
>> anywhere between hard_header and send time.
> 
> IB wants to pass addressing information between layers, it needs to
> find a safe way to do that.

We *had* a safe way to do that.  It got broken.  What about increasing
the size of skb->cb?  Or adding a skb->dgid that is a
u8[INFINIBAND_ALEN]?  Or a more generic skb->dest_ll_addr that is sized
to hold the dest address for any link layer?

> Pushing metadata before the head of the SKB data pointer is illegal,
> as the layers in between might want to push protocol headers,

That's a total non-issue for us.  There are no headers that protocols
can add before ours.

> mirror
> the packet to another interface

Doesn't that then mean that the headers specific to this interface
should be stripped before the mirror?  If so, I believe the way Paolo
did this patch, that is what will be done.

>, etc.
> 
> So this "metadata in SKB data" approach is buggy too.




-- 
Doug Ledford <dledford@...hat.com>
    GPG Key ID: 0E572FDD



Download attachment "signature.asc" of type "application/pgp-signature" (885 bytes)