From:   "Bhandare, KiranX" <kiranx.bhandare@intel.com>
To:     "Lobakin, Alexandr" <alexandr.lobakin@intel.com>,
        "intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>
CC:     Song Liu <songliubraving@fb.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Yonghong Song <yhs@fb.com>, Martin KaFai Lau <kafai@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        "Andrii Nakryiko" <andrii@kernel.org>,
        =?iso-8859-1?Q?Bj=F6rn_T=F6pel?= <bjorn@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "Jakub Kicinski" <kuba@kernel.org>, KP Singh <kpsingh@kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [Intel-wired-lan] [PATCH v4 net-next 0/9] net: intel:
 napi_alloc_skb() vs metadata
Thread-Topic: [Intel-wired-lan] [PATCH v4 net-next 0/9] net: intel:
 napi_alloc_skb() vs metadata
Thread-Index: AQHX7D0GBStG13BisUmYcc0eBW8cqqxcPCkA
Date:   Mon, 10 Jan 2022 10:11:43 +0000
Message-ID: <DM6PR11MB3292AA2C0D33C635FE9D068BF1509@DM6PR11MB3292.namprd11.prod.outlook.com>
References: <20211208140702.642741-1-alexandr.lobakin@intel.com>
In-Reply-To: <20211208140702.642741-1-alexandr.lobakin@intel.com>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.6.200.16
dlp-product: dlpe-windows
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?bB+DWYPCNu+OGKfEt/RCuKZ8BJCZaDMhqLzRC6ME6QFkm9rAPfiliGRjgF?=
 =?iso-8859-1?Q?aw2hRbz9b1O1aMEf9Z6fFfsrDkOwcqdPqOw4KfxYHkMywKgLjf0g8ZQ6db?=
 =?iso-8859-1?Q?85iYiov3KSv/9a9MlL3nxo9qfWtlPvVeSJiTmdKhDB5t/w8U4jGYL6cccN?=
 =?iso-8859-1?Q?TYiYaoe5Gtj7dOC789Ub91Edxs5nAmaqjHPY+dR5NdUtnvWHYXba67IaUp?=
 =?iso-8859-1?Q?HI+XoD3q9zucPGphfbt5WadzmaHojsESCeZx+UoHHhb4UXuocQnb9ior2T?=
 =?iso-8859-1?Q?ZGS5R3nVAO+EknFRTc5j7MDpkj1XSj7JufdqzhZQA4EWlwLzFRDU2fqfLg?=
 =?iso-8859-1?Q?G1bamMwhyYh7q+4iZdBlJZLFQdcqSGFGoZgKb+Clbj9y5vkYMOeSliraeO?=
 =?iso-8859-1?Q?GaftfXkcINj6VAYbLSJnozyZk/3cuCCoIgKzkLH1iKZ5U2Bc4i9XF9k6ox?=
 =?iso-8859-1?Q?navqQ8VEDsMa8JMCfraAu4mwoidiOKthScorVhheZF6sJDtHRMlmIJyIEK?=
 =?iso-8859-1?Q?nwRr0aak9PTj2eh3slMsa+rB2FmbNemzmD1Glts/alYu/OQuQU7ksGfKeX?=
 =?iso-8859-1?Q?VDgfwEDibSByTWMF5zGaL7dFfDQrcWJ6TLkkQ1TCduEaT985QxM4CuO/90?=
 =?iso-8859-1?Q?eUL8pyyVv4qh+iO+SbnYqv3JxXerCSbDKrqEg5QfA2NQPd9Y+9Y2udsAGG?=
 =?iso-8859-1?Q?cVS6MbajUDOtBOo+QEE09kl4VfIwX5QaWJGS9eaU0ukBT0qONNGMyEIjPZ?=
 =?iso-8859-1?Q?OASiIp//VuxmtSnmyqk8wYWJ9sMBCJ05gEJGdSNxuBpJjPyQIG24hUwILj?=
 =?iso-8859-1?Q?joAiPGN8nnRD76QKo/byOY0+uLOKeWJHeY1JCgO4UJz4Gfkbeb2FIoBXP4?=
 =?iso-8859-1?Q?3pKLirfBBMXzG+1RG++bP6Ev2VsxC6ZvBbC1d/5rraL0ARFqY4gJz4q7Ky?=
 =?iso-8859-1?Q?rUsQr7KkbYOKgNRLQbuac6HfOz3sJMsbWgHcyFgOf76qPwOEdMDqk0HCn9?=
 =?iso-8859-1?Q?8NBBJI74by5IESW1sLF6zJ18fU7ReK33ISubWM8W1h6MVsun1ZD57b4W4A?=
 =?iso-8859-1?Q?QuJFRTfDoljCsQhVUn3DSvR+GJGUc+OVII312W3Ab9ckzpFiDbB9YCcYem?=
 =?iso-8859-1?Q?zrxA2UpqPPjR1ETxpy3kCH0kJUnND4XD3otYLQhfTS/Ny4x3AkLYBiLVzi?=
 =?iso-8859-1?Q?Sn0ha7WJUxyMdKkwFOWOhnJBm+Ibb+Z7BS0evkxDhPQ1XRMfruc2h4V58d?=
 =?iso-8859-1?Q?ABc+v6obATfiXlbVO3HTCeC/3Ux+xWb3PxHcr/ni+UGEVRvSN8Stk3ZFAe?=
 =?iso-8859-1?Q?ggPZJqiDtfiX7QhVgC44C41rhJykKyqeUP/yIXqQ6zJ9HKKjbBTipy/eyw?=
 =?iso-8859-1?Q?21n+CJHV1CUcrIHHfdk/NkzV5tzKdPorYp1t12i3SS84b0w32A44+ZccwO?=
 =?iso-8859-1?Q?AaehCHPEBYSfPniAW/p4n9I001HVN9seSlI3vnhie401chIXCyzGU/5zxc?=
 =?iso-8859-1?Q?2BzYWAjmvBH5mxakVq7S7yJl5qFLPJ/f4pkvU6LD6U93Gkna12TTmHo3oy?=
 =?iso-8859-1?Q?dCl5Srqfdp0hkz/8QUZuZf9L2aEgH/AOZoMU1GXrpzbTScG8KhyQ0MRgd/?=
 =?iso-8859-1?Q?2zkXutcAjoHQMuYxLbQf4kHStE0dT0o+IFzjsEUABGGXBggIJLTCtC8WPL?=
 =?iso-8859-1?Q?gZfNiXMCdoTGlbW+vU6k1NIs1mqb/5iQriPSunks/yd/m62Oo0FyujA/qu?=
 =?iso-8859-1?Q?thdQ=3D=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3292.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bac306f0-975c-4230-86c7-08d9d4219a61
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jan 2022 10:11:43.8102
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: GdExyaBiYwy+jgf3g4gumVHT8rbhRpj4DJ32D8xsCnBxxjWf9TWP+DogXjWPWW3MgF18Vy4/3bx2mO5gV7ZZ4to5gjOaNlFWwqLOlLJXJzY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4153
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Alexander Lobakin
> Sent: Wednesday, December 8, 2021 7:37 PM
> To: intel-wired-lan@lists.osuosl.org
> Cc: Song Liu <songliubraving@fb.com>; Jesper Dangaard Brouer
> <hawk@kernel.org>; Daniel Borkmann <daniel@iogearbox.net>; Yonghong
> Song <yhs@fb.com>; Martin KaFai Lau <kafai@fb.com>; John Fastabend
> <john.fastabend@gmail.com>; Alexei Starovoitov <ast@kernel.org>; Andrii
> Nakryiko <andrii@kernel.org>; Bj=F6rn T=F6pel <bjorn@kernel.org>;
> netdev@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>; KP Singh
> <kpsingh@kernel.org>; bpf@vger.kernel.org; David S. Miller
> <davem@davemloft.net>; linux-kernel@vger.kernel.org
> Subject: [Intel-wired-lan] [PATCH v4 net-next 0/9] net: intel: napi_alloc=
_skb()
> vs metadata
>=20
> This is an interpolation of [0] to other Intel Ethernet drivers (and is (=
re)based
> on its code).
> The main aim is to keep XDP metadata not only in case with build_skb(), b=
ut
> also when we do napi_alloc_skb() + memcpy().
>=20
> All Intel drivers suffers from the same here:
>  - metadata gets lost on XDP_PASS in legacy-rx;
>  - excessive headroom allocation on XSK Rx to skbs;
>  - metadata gets lost on XSK Rx to skbs.
>=20
> Those get especially actual in XDP Hints upcoming.
> I couldn't have addressed the first one for all Intel drivers due to that=
 they
> don't reserve any headroom for now in legacy-rx mode even with XDP
> enabled. This is hugely wrong, but requires quite a bunch of work and a
> separate series. Luckily, ice doesn't suffer from that.
> igc has 1 and 3 already fixed in [0].
>=20
> From v3 ([1]):
>  - fix driver name and ixgbe_construct_skb() function name in the
>    commit message of #9 (Jesper);
>  - no functional changes.
>=20
> From v2 (unreleased upstream):
>  - tweaked 007 to pass bi->xdp directly and simplify code (Maciej);
>  - picked Michal's Reviewed-by.
>=20
> From v1 (unreleased upstream):
>  - drop "fixes" of legacy-rx for i40e, igb and ixgbe since they have
>    another flaw regarding headroom (see above);
>  - drop igc cosmetic fixes since they landed upstream incorporated
>    into Jesper's commits;
>  - picked one Acked-by from Maciej.
>=20
> [0]
> https://lore.kernel.org/netdev/163700856423.565980.101625649213476937
> 58.stgit@firesoul
> [1] https://lore.kernel.org/netdev/20211207205536.563550-1-
> alexandr.lobakin@intel.com
>=20
> Alexander Lobakin (9):
>   i40e: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>   i40e: respect metadata on XSK Rx to skb
>   ice: respect metadata in legacy-rx/ice_construct_skb()
>   ice: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>   ice: respect metadata on XSK Rx to skb
>   igc: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>   ixgbe: pass bi->xdp to ixgbe_construct_skb_zc() directly
>   ixgbe: don't reserve excessive XDP_PACKET_HEADROOM on XSK Rx to skb
>   ixgbe: respect metadata on XSK Rx to skb
>=20
>  drivers/net/ethernet/intel/i40e/i40e_xsk.c   | 16 +++++++-----
>  drivers/net/ethernet/intel/ice/ice_txrx.c    | 15 ++++++++---
>  drivers/net/ethernet/intel/ice/ice_xsk.c     | 16 +++++++-----
>  drivers/net/ethernet/intel/igc/igc_main.c    | 13 +++++-----
>  drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 27 ++++++++++++--------
>  5 files changed, 54 insertions(+), 33 deletions(-)
>=20

Tested-by: Kiran Bhandare <kiranx.bhandare@intel.com>  A Contingent Worker =
at Intel