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