On 10/13/2016 10:43 AM, David Miller wrote:
> From: Doug Ledford <dledford@xxxxxxxxxx>
> Date: Thu, 13 Oct 2016 10:35:35 -0400
>
>> On 10/13/2016 10:24 AM, David Miller wrote:
>>> From: Paolo Abeni <pabeni@xxxxxxxxxx>
>>> 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@xxxxxxxxxx>
>>>
>>> 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@xxxxxxxxxx>
GPG Key ID: 0E572FDD
Attachment:
signature.asc
Description: OpenPGP digital signature
