From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 65A7E42C10; Fri, 2 Jun 2023 10:46:33 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3B78240ED8; Fri, 2 Jun 2023 10:46:33 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 1C614406B8 for ; Fri, 2 Jun 2023 10:46:32 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 89FA85A; Fri, 2 Jun 2023 11:46:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 89FA85A DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1685695591; bh=13x2E8hyc5a2AsBHxBWUOTow3uLwbl5HF4zoJVSbtGU=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=PD9Snui/VZIscBcMq8rNjLRgAhm11iMvhTNagbu790KUwBDAyPFwvwe02RRNYTrUf Jx/ARlqpCjz4EeRyngINAFsMxLfsxi4gWwTZc4paHC5tRM0TJ3kJas1rSh5c6uXHC3 btry9daw79pjMHhqcN60lcIeyR9YDzymRQaxXKUU= Message-ID: Date: Fri, 2 Jun 2023 11:46:31 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH v4 3/3] net/sfc: support VLAN stripping offload Content-Language: en-US To: Artemii Morozov , dev@dpdk.org Cc: Ivan Malov , Viacheslav Galaktionov , Andy Moreton References: <20230531134122.119508-1-artemii.morozov@arknetworks.am> <20230601153022.99634-1-artemii.morozov@arknetworks.am> <20230601153022.99634-4-artemii.morozov@arknetworks.am> From: Andrew Rybchenko Organization: OKTET Labs In-Reply-To: <20230601153022.99634-4-artemii.morozov@arknetworks.am> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 6/1/23 18:30, Artemii Morozov wrote: > Extract vlan tci provided by the HW in the prefix and put it to mbuf. vlan -> VLAN, tci -> TCI > VLAN stripping is supported for ef100 datapath only. It should be highlighted that it is device level offload. > > Signed-off-by: Artemii Morozov > Reviewed-by: Viacheslav Galaktionov > Reviewed-by: Ivan Malov > Reviewed-by: Andy Moreton > --- > doc/guides/nics/sfc_efx.rst | 4 ++-- > drivers/net/sfc/sfc_ef100_rx.c | 21 ++++++++++++++++++++- > drivers/net/sfc/sfc_rx.c | 19 +++++++++++++++++++ Release notes should be updated to advertise the feature. > 3 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/doc/guides/nics/sfc_efx.rst b/doc/guides/nics/sfc_efx.rst > index de0656876b..44fa24e1ba 100644 > --- a/doc/guides/nics/sfc_efx.rst > +++ b/doc/guides/nics/sfc_efx.rst > @@ -118,6 +118,8 @@ SFC EFX PMD has support for: > > - Port representors (see :ref: switch_representation) > > +- VLAN stripping (if running firmware variant supports it) > + > > Non-supported Features > ---------------------- > @@ -132,8 +134,6 @@ The features not yet supported include: > > - VLAN filtering > > -- VLAN stripping > - > - LRO > > > diff --git a/drivers/net/sfc/sfc_ef100_rx.c b/drivers/net/sfc/sfc_ef100_rx.c > index 37b754fa33..e323156a26 100644 > --- a/drivers/net/sfc/sfc_ef100_rx.c > +++ b/drivers/net/sfc/sfc_ef100_rx.c > @@ -68,6 +68,7 @@ struct sfc_ef100_rxq { > #define SFC_EF100_RXQ_INGRESS_MPORT 0x80 > #define SFC_EF100_RXQ_USER_FLAG 0x100 > #define SFC_EF100_RXQ_NIC_DMA_MAP 0x200 > +#define SFC_EF100_RXQ_VLAN_STRIP 0x400 > unsigned int ptr_mask; > unsigned int evq_phase_bit_shift; > unsigned int ready_pkts; > @@ -392,6 +393,7 @@ static const efx_rx_prefix_layout_t sfc_ef100_rx_prefix_layout = { > SFC_EF100_RX_PREFIX_FIELD(RSS_HASH, B_FALSE), > SFC_EF100_RX_PREFIX_FIELD(USER_FLAG, B_FALSE), > SFC_EF100_RX_PREFIX_FIELD(USER_MARK, B_FALSE), > + SFC_EF100_RX_PREFIX_FIELD(VLAN_STRIP_TCI, B_FALSE), > > #undef SFC_EF100_RX_PREFIX_FIELD > } > @@ -472,6 +474,17 @@ sfc_ef100_rx_prefix_to_offloads(const struct sfc_ef100_rxq *rxq, > ESF_GZ_RX_PREFIX_INGRESS_MPORT); > } > > + if (rxq->flags & SFC_EF100_RXQ_VLAN_STRIP) { > + uint32_t vlan_stripped; Please, add empty line after variable declaration. IMHO, bool type should be used here. > + vlan_stripped = EFX_XWORD_FIELD(rx_prefix[0], ESF_GZ_RX_PREFIX_VLAN_STRIPPED); > + > + if (vlan_stripped != 0) { No comparison if bool is used. > + ol_flags |= RTE_MBUF_F_RX_VLAN | RTE_MBUF_F_RX_VLAN_STRIPPED; > + m->vlan_tci = EFX_XWORD_FIELD(rx_prefix[0], > + ESF_GZ_RX_PREFIX_VLAN_STRIP_TCI); > + } > + } > + > m->ol_flags = ol_flags; > return true; > } > @@ -892,6 +905,12 @@ sfc_ef100_rx_qstart(struct sfc_dp_rxq *dp_rxq, unsigned int evq_read_ptr, > (rxq->flags & SFC_EF100_RXQ_INGRESS_MPORT)) > return ENOTSUP; > > + if ((unsup_rx_prefix_fields & > + (1U << EFX_RX_PREFIX_FIELD_VLAN_STRIP_TCI)) == 0) Shouldn't offload enable/disable be taken into account here? If offload is not enabled, it is better to skip extra read from Rx prefix and branching on fast path. > + rxq->flags |= SFC_EF100_RXQ_VLAN_STRIP; > + else > + rxq->flags &= ~SFC_EF100_RXQ_VLAN_STRIP; > + > rxq->prefix_size = pinfo->erpl_length; > rxq->rearm_data = sfc_ef100_mk_mbuf_rearm_data(rxq->dp.dpq.port_id, > rxq->prefix_size); > @@ -1004,7 +1023,7 @@ struct sfc_dp_rx sfc_ef100_rx = { > SFC_DP_RX_FEAT_FLOW_MARK | > SFC_DP_RX_FEAT_INTR | > SFC_DP_RX_FEAT_STATS, > - .dev_offload_capa = 0, > + .dev_offload_capa = RTE_ETH_RX_OFFLOAD_VLAN_STRIP, > .queue_offload_capa = RTE_ETH_RX_OFFLOAD_CHECKSUM | > RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM | > RTE_ETH_RX_OFFLOAD_OUTER_UDP_CKSUM | > diff --git a/drivers/net/sfc/sfc_rx.c b/drivers/net/sfc/sfc_rx.c > index edd0f0c038..e9ef1d92ed 100644 > --- a/drivers/net/sfc/sfc_rx.c > +++ b/drivers/net/sfc/sfc_rx.c > @@ -938,6 +938,9 @@ sfc_rx_get_offload_mask(struct sfc_adapter *sa) > if (encp->enc_tunnel_encapsulations_supported == 0) > no_caps |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM; > > + if (encp->enc_rx_vlan_stripping == 0) > + no_caps |= RTE_ETH_RX_OFFLOAD_VLAN_STRIP; > + > return ~no_caps; > } > > @@ -1186,6 +1189,16 @@ sfc_rx_qinit(struct sfc_adapter *sa, sfc_sw_index_t sw_index, > if (offloads & RTE_ETH_RX_OFFLOAD_RSS_HASH) > rxq_info->type_flags |= EFX_RXQ_FLAG_RSS_HASH; > > + Too many empty lines > + if (sa->eth_dev->data->dev_conf.rxmode.offloads & > + RTE_ETH_RX_OFFLOAD_VLAN_STRIP) { > + rxq_info->type_flags |= EFX_RXQ_FLAG_VLAN_STRIP; > + } else if (rx_conf->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP) { > + sfc_err(sa, "VLAN stripping must be configured during device configure"); > + rc = EINVAL; > + goto fail_bad_conf; As far as I know generic ethdev code will reject the request earlier. So, the code is unreachable and dead. > + } > + > if ((sa->negotiated_rx_metadata & RTE_ETH_RX_METADATA_USER_FLAG) != 0) > rxq_info->type_flags |= EFX_RXQ_FLAG_USER_FLAG; > > @@ -1691,6 +1704,12 @@ sfc_rx_check_mode(struct sfc_adapter *sa, struct rte_eth_rxmode *rxmode) > rxmode->offloads |= RTE_ETH_RX_OFFLOAD_OUTER_IPV4_CKSUM; > } > > + if ((rxmode->offloads & RTE_ETH_RX_OFFLOAD_VLAN_STRIP) && > + (~offloads_supported & RTE_ETH_RX_OFFLOAD_VLAN_STRIP)) { > + sfc_err(sa, "VLAN stripping offload is requested but not supported"); > + rc = ENOTSUP; > + } > + If I'm not mistaken generic ethdev code will reject the request earlier and will not allow to reach the code here. > return rc; > } >