Message ID | 20230512152607.992209-11-larysa.zaremba@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp5213948vqo; Fri, 12 May 2023 08:58:13 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4PJVIFqO/w5Qt7iaa7XMEDOMm6oaSqX2+IWarJm4wJ9p+EWc3WLIRxqkd/AqmHo+PigkF8 X-Received: by 2002:a05:6a20:7489:b0:f6:4c57:265d with SMTP id p9-20020a056a20748900b000f64c57265dmr31077987pzd.1.1683907092810; Fri, 12 May 2023 08:58:12 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1683907092; cv=none; d=google.com; s=arc-20160816; b=bdAFT+lFPwii4N0nEP+avjf7p5G73FVkv0gj/TCd2M0Tkma6Qg/6EWir8QReoHwLQa EXPCCvFqCd3HPCcQiwNbIydlTMaR67RgBYQKpwVIH9AeNCijCVmqr1PZ2SwLe7+gKgeY JHODPnab2iKizirtzWOZZUDRO4zjp6FJUOcJ4rEJ9D4s8+VT+xH2BuD2bVCpKl56c9ws LB3ucu+arDQ+uMj0eHtwpQo8a7jCnLzEVi6vb9BQCMjmNjZJWbwl4W8gk0AZ7GNjd5rR K+RHZ3QvmofZPKNwopYuduX1r5Edwi9Ee2EmwNY8jjoXCgUHVwHT6WKoRLsxx3EchVJx cyRQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=a7M/o1kHU1gcxVwkFj/XYxriycfCGZA6ZU4Lgo3xVyA=; b=I9euxhFkJs/1NvKyfHIBpQb5XM2BdBPrOW/uFMl7ZSKzi6LH94vA4oOC81Wlrm3ZSp /XrZUIx7+pc/Vjlb4it55ssmmShvfPKjqKJxoCC4UeK0PTGTYR6EPGA1igIZT4pKKbk6 xCDwSlbdUEX4ZeHX1xUBJQgAcmn0Kj6f1LcmFkDZH6elpL+lYr+Ok70zaWFlrK3YrzFc 1KsY3pK5rAaC0W/iaNu82xENDcBsegoI5TUk4+EoV722TRPyAhp6vfoc0Qxbi4R7NVJZ Icw/NPPJ0gJpwmsvSyGBx43tE1nDa0rvwFBpV12uvBk3rTXgX2DXffc8VPE3hao46Vid jgyw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=PQh9pB89; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id r71-20020a632b4a000000b0051f6974b6f9si10323177pgr.789.2023.05.12.08.58.00; Fri, 12 May 2023 08:58:12 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=PQh9pB89; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241903AbjELP37 (ORCPT <rfc822;peekingduck44@gmail.com> + 99 others); Fri, 12 May 2023 11:29:59 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47084 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241964AbjELP3U (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 12 May 2023 11:29:20 -0400 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 791DC12A; Fri, 12 May 2023 08:29:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1683905343; x=1715441343; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=Ca3C3CN5XD0jYqPpaEXrBQ6QBGcOSKPhdaJMaphgwXU=; b=PQh9pB89PXhnr1uNHS/rreEYL0FBokJLmKvrBgjlhncDlC2z61AUUpwo hETMj22BIRMaZoRbXstbrqM0cxWPmzJm/+eT1nynQK453xPwEeCbf8fjL i9UUZRV6IJc6vBPrTRYvMUwBD94SEzszvPkQ7aWB8qciq0gstUv97KjG0 43ZdytZWMlPyVdKeQyK7VkcwFVXSEy46y8xzldxQX4lIgOEi3g5vTDWa6 lsU7F31iiz2YvSI05aNuql4VhX/k96oGOS+4EAnYNjYGWnJBWIm6voii9 mxtPFCQrrGIADDWT+Um0PEVuHbi49NJMRdJog/b/w0bpt9jky44Tm8UlO w==; X-IronPort-AV: E=McAfee;i="6600,9927,10708"; a="349653405" X-IronPort-AV: E=Sophos;i="5.99,269,1677571200"; d="scan'208";a="349653405" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 12 May 2023 08:28:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10708"; a="1030124568" X-IronPort-AV: E=Sophos;i="5.99,269,1677571200"; d="scan'208";a="1030124568" Received: from irvmail002.ir.intel.com ([10.43.11.120]) by fmsmga005.fm.intel.com with ESMTP; 12 May 2023 08:28:54 -0700 Received: from lincoln.igk.intel.com (lincoln.igk.intel.com [10.102.21.235]) by irvmail002.ir.intel.com (Postfix) with ESMTP id 1EF1A35FB7; Fri, 12 May 2023 16:28:53 +0100 (IST) From: Larysa Zaremba <larysa.zaremba@intel.com> To: bpf@vger.kernel.org Cc: Larysa Zaremba <larysa.zaremba@intel.com>, Stanislav Fomichev <sdf@google.com>, Alexei Starovoitov <ast@kernel.org>, Daniel Borkmann <daniel@iogearbox.net>, Andrii Nakryiko <andrii@kernel.org>, Jakub Kicinski <kuba@kernel.org>, Martin KaFai Lau <martin.lau@linux.dev>, Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>, John Fastabend <john.fastabend@gmail.com>, KP Singh <kpsingh@kernel.org>, Jiri Olsa <jolsa@kernel.org>, Jesse Brandeburg <jesse.brandeburg@intel.com>, Tony Nguyen <anthony.l.nguyen@intel.com>, Anatoly Burakov <anatoly.burakov@intel.com>, Jesper Dangaard Brouer <brouer@redhat.com>, Alexander Lobakin <alexandr.lobakin@intel.com>, Magnus Karlsson <magnus.karlsson@gmail.com>, Maryam Tahhan <mtahhan@redhat.com>, xdp-hints@xdp-project.net, netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH RESEND bpf-next 10/15] ice: Implement VLAN tag hint Date: Fri, 12 May 2023 17:26:02 +0200 Message-Id: <20230512152607.992209-11-larysa.zaremba@intel.com> X-Mailer: git-send-email 2.35.3 In-Reply-To: <20230512152607.992209-1-larysa.zaremba@intel.com> References: <20230512152607.992209-1-larysa.zaremba@intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1765704563859720504?= X-GMAIL-MSGID: =?utf-8?q?1765704563859720504?= |
Series |
new kfunc XDP hints and ice implementation
|
|
Commit Message
Larysa Zaremba
May 12, 2023, 3:26 p.m. UTC
Implement .xmo_rx_vlan_tag callback to allow XDP code to read
packet's VLAN tag.
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 44 +++++++++++++++++++
1 file changed, 44 insertions(+)
Comments
On 05/12, Larysa Zaremba wrote: > Implement .xmo_rx_vlan_tag callback to allow XDP code to read > packet's VLAN tag. > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > --- > drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 44 +++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > index 1caa73644e7b..39547feb6106 100644 > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > @@ -627,7 +627,51 @@ static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash, > return 0; > } > > +/** > + * ice_xdp_rx_ctag - VLAN tag XDP hint handler > + * @ctx: XDP buff pointer > + * @vlan_tag: destination address > + * > + * Copy VLAN tag (if was stripped) to the destination address. > + */ > +static int ice_xdp_rx_ctag(const struct xdp_md *ctx, u16 *vlan_tag) > +{ > + const struct ice_xdp_buff *xdp_ext = (void *)ctx; > + netdev_features_t features; > + [..] > + features = xdp_ext->rx_ring->netdev->features; > + > + if (!(features & NETIF_F_HW_VLAN_CTAG_RX)) > + return -EINVAL; Passing-by comment: why do we need to check features? ice_get_vlan_tag_from_rx_desc seems to be checking a bunch of fields in the descriptors, so that should be enough? > + > + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); Should we also do the following: if (!*vlan_tag) return -ENODATA; ? > + return 0; > +} > + > +/** > + * ice_xdp_rx_stag - VLAN s-tag XDP hint handler > + * @ctx: XDP buff pointer > + * @vlan_tag: destination address > + * > + * Copy VLAN s-tag (if was stripped) to the destination address. > + */ > +static int ice_xdp_rx_stag(const struct xdp_md *ctx, u16 *vlan_tag) > +{ > + const struct ice_xdp_buff *xdp_ext = (void *)ctx; > + netdev_features_t features; > + > + features = xdp_ext->rx_ring->netdev->features; > + > + if (!(features & NETIF_F_HW_VLAN_STAG_RX)) > + return -EINVAL; > + > + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); > + return 0; > +} > + > const struct xdp_metadata_ops ice_xdp_md_ops = { > .xmo_rx_timestamp = ice_xdp_rx_hw_ts, > .xmo_rx_hash = ice_xdp_rx_hash, > + .xmo_rx_ctag = ice_xdp_rx_ctag, > + .xmo_rx_stag = ice_xdp_rx_stag, > }; > -- > 2.35.3 >
On Fri, May 12, 2023 at 11:31:21AM -0700, Stanislav Fomichev wrote: > On 05/12, Larysa Zaremba wrote: > > Implement .xmo_rx_vlan_tag callback to allow XDP code to read > > packet's VLAN tag. > > > > Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com> > > --- > > drivers/net/ethernet/intel/ice/ice_txrx_lib.c | 44 +++++++++++++++++++ > > 1 file changed, 44 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > > index 1caa73644e7b..39547feb6106 100644 > > --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > > +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c > > @@ -627,7 +627,51 @@ static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash, > > return 0; > > } > > > > +/** > > + * ice_xdp_rx_ctag - VLAN tag XDP hint handler > > + * @ctx: XDP buff pointer > > + * @vlan_tag: destination address > > + * > > + * Copy VLAN tag (if was stripped) to the destination address. > > + */ > > +static int ice_xdp_rx_ctag(const struct xdp_md *ctx, u16 *vlan_tag) > > +{ > > + const struct ice_xdp_buff *xdp_ext = (void *)ctx; > > + netdev_features_t features; > > + > > [..] > > > + features = xdp_ext->rx_ring->netdev->features; > > + > > + if (!(features & NETIF_F_HW_VLAN_CTAG_RX)) > > + return -EINVAL; > > Passing-by comment: why do we need to check features? > ice_get_vlan_tag_from_rx_desc seems to be checking a bunch of > fields in the descriptors, so that should be enough? Unfortunately, it is not enough, because it only checks, if there is a valid value in the descriptor, without distinguishing c-tag from s-tag. In this hardware, c-tag and s-tag are mutually exclusive, so they can occupy same descriptor fields. Checking netdev features is just the easiest way to tell them apart. I guess, storing this information in in the ring structure would be more efficient than checking netdev features. I know Piotr Raczynski indends to review this series, so maybe he would provide some additional feedback/suggestions. > > > + > > + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); > > Should we also do the following: > > if (!*vlan_tag) > return -ENODATA; > > ? Oh, returning VLAN tag with zero value really made sense to me at the beginning, but after playing with different kinds of packets, I think returning error makes more sense. Will change. > > > + return 0; > > +} > > + > > +/** > > + * ice_xdp_rx_stag - VLAN s-tag XDP hint handler > > + * @ctx: XDP buff pointer > > + * @vlan_tag: destination address > > + * > > + * Copy VLAN s-tag (if was stripped) to the destination address. > > + */ > > +static int ice_xdp_rx_stag(const struct xdp_md *ctx, u16 *vlan_tag) > > +{ > > + const struct ice_xdp_buff *xdp_ext = (void *)ctx; > > + netdev_features_t features; > > + > > + features = xdp_ext->rx_ring->netdev->features; > > + > > + if (!(features & NETIF_F_HW_VLAN_STAG_RX)) > > + return -EINVAL; > > + > > + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); > > + return 0; > > +} > > + > > const struct xdp_metadata_ops ice_xdp_md_ops = { > > .xmo_rx_timestamp = ice_xdp_rx_hw_ts, > > .xmo_rx_hash = ice_xdp_rx_hash, > > + .xmo_rx_ctag = ice_xdp_rx_ctag, > > + .xmo_rx_stag = ice_xdp_rx_stag, > > }; > > -- > > 2.35.3 > >
On 15/05/2023 15.41, Larysa Zaremba wrote: >>> + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); >> Should we also do the following: >> >> if (!*vlan_tag) >> return -ENODATA; >> >> ? > Oh, returning VLAN tag with zero value really made sense to me at the beginning, > but after playing with different kinds of packets, I think returning error makes > more sense. Will change. > IIRC then VLAN tag zero is also a valid id, right? --Jesper
On Mon, May 15, 2023 at 05:07:19PM +0200, Jesper Dangaard Brouer wrote: > > > On 15/05/2023 15.41, Larysa Zaremba wrote: > > > > + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); > > > Should we also do the following: > > > > > > if (!*vlan_tag) > > > return -ENODATA; > > > > > > ? > > Oh, returning VLAN tag with zero value really made sense to me at the beginning, > > but after playing with different kinds of packets, I think returning error makes > > more sense. Will change. > > > > IIRC then VLAN tag zero is also a valid id, right? AFAIK, 0x000 is reseved and basically means "no vlan tag". When ice hardware returns such value in descriptor, it says "no vlan tag was stripped" and this doesn't necessarily mean there is no VLAN tag in the packet. For example, let us consider a packet: Ether/802.1ad(s-tag)/802.1q(c-tag)/... Hardware does not strip c-tag in such case and sends 0x000 in the descriptor, but packet clearly does contain a c-tag, so at least in ice, it is reasonable to not consider '0' a reliable value. I guess, for s-tag value of 0x000 should be more reliable, so maybe 'if (!*vlan_tag)' usage can be limited to c-tag function. > > --Jesper >
diff --git a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c index 1caa73644e7b..39547feb6106 100644 --- a/drivers/net/ethernet/intel/ice/ice_txrx_lib.c +++ b/drivers/net/ethernet/intel/ice/ice_txrx_lib.c @@ -627,7 +627,51 @@ static int ice_xdp_rx_hash(const struct xdp_md *ctx, u32 *hash, return 0; } +/** + * ice_xdp_rx_ctag - VLAN tag XDP hint handler + * @ctx: XDP buff pointer + * @vlan_tag: destination address + * + * Copy VLAN tag (if was stripped) to the destination address. + */ +static int ice_xdp_rx_ctag(const struct xdp_md *ctx, u16 *vlan_tag) +{ + const struct ice_xdp_buff *xdp_ext = (void *)ctx; + netdev_features_t features; + + features = xdp_ext->rx_ring->netdev->features; + + if (!(features & NETIF_F_HW_VLAN_CTAG_RX)) + return -EINVAL; + + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); + return 0; +} + +/** + * ice_xdp_rx_stag - VLAN s-tag XDP hint handler + * @ctx: XDP buff pointer + * @vlan_tag: destination address + * + * Copy VLAN s-tag (if was stripped) to the destination address. + */ +static int ice_xdp_rx_stag(const struct xdp_md *ctx, u16 *vlan_tag) +{ + const struct ice_xdp_buff *xdp_ext = (void *)ctx; + netdev_features_t features; + + features = xdp_ext->rx_ring->netdev->features; + + if (!(features & NETIF_F_HW_VLAN_STAG_RX)) + return -EINVAL; + + *vlan_tag = ice_get_vlan_tag_from_rx_desc(xdp_ext->eop_desc); + return 0; +} + const struct xdp_metadata_ops ice_xdp_md_ops = { .xmo_rx_timestamp = ice_xdp_rx_hw_ts, .xmo_rx_hash = ice_xdp_rx_hash, + .xmo_rx_ctag = ice_xdp_rx_ctag, + .xmo_rx_stag = ice_xdp_rx_stag, };