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 A6081A0C45; Wed, 22 Sep 2021 10:01:30 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 3AFF041196; Wed, 22 Sep 2021 10:01:30 +0200 (CEST) Received: from smtpbg501.qq.com (smtpbg501.qq.com [203.205.250.101]) by mails.dpdk.org (Postfix) with ESMTP id 26F3E4069E for ; Wed, 22 Sep 2021 10:01:27 +0200 (CEST) X-QQ-mid: bizesmtp45t1632297681tb3p3nek Received: from jiawenwu (unknown [183.129.236.74]) by esmtp6.qq.com (ESMTP) with id ; Wed, 22 Sep 2021 16:01:20 +0800 (CST) X-QQ-SSF: 01400000002000E0G000B00A0000000 X-QQ-FEAT: FVl8EHhfVR6QgDH3KYLtFfxD/jMuML0ki0fnVQoS9j94za2cqIcu0huK51oPS +rc0GfEXfGyIVnmNx29J4dV+TXtwzCqMIbXTMUxULNGCYKozapQtPyEqBIuyCS9K17ywid9 WZMyN3gRSd4kG+0zxeamj7diGzInikrr9iaseUjyHoAnUZZ53NV06wODwTXWT0UUq1J+29s lcmvlyhSkWThJ5Y8V8tAFOYbmnpLVB1C8KtHkhJyJa0K6z8Ek+HYbQpSmQAQfjK8iIOEXZq e9fh0WuiHECOK7t3p4eoZh0oRw0vBHn1TQ6/xpql120rsQS4AJZ/GCkZr84u7cUIbitZxcv zG1GNFDQyoo/iUBhLZV0W4swaIYKQ== X-QQ-GoodBg: 2 From: "Jiawen Wu" To: "'Ferruh Yigit'" , References: <20210908083758.312055-1-jiawenwu@trustnetic.com> <20210908083758.312055-2-jiawenwu@trustnetic.com> In-Reply-To: Date: Wed, 22 Sep 2021 16:01:20 +0800 Message-ID: <004801d7af88$08249ff0$186ddfd0$@trustnetic.com>+EFB683DA12095ADC MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIOtMP2PGxNII75rhGYQ2F9RaTVpgKR23I4AeR+75qrHecyAA== Content-Language: zh-cn X-QQ-SENDSIZE: 520 Feedback-ID: bizesmtp:trustnetic.com:qybgforeign:qybgforeign6 X-QQ-Bgrelay: 1 Subject: Re: [dpdk-dev] [PATCH 01/32] net/ngbe: add packet type 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 Sender: "dev" On September 16, 2021 12:48 AM, Ferruh Yigit wrote: > On 9/8/2021 9:37 AM, Jiawen Wu wrote: > > Add packet type marco definition and convert ptype to ptid. > > > > Signed-off-by: Jiawen Wu > > --- > > doc/guides/nics/features/ngbe.ini | 1 + > > doc/guides/nics/ngbe.rst | 1 + > > drivers/net/ngbe/meson.build | 1 + > > drivers/net/ngbe/ngbe_ethdev.c | 9 + > > drivers/net/ngbe/ngbe_ethdev.h | 4 + > > drivers/net/ngbe/ngbe_ptypes.c | 300 > ++++++++++++++++++++++++++++++ > > drivers/net/ngbe/ngbe_ptypes.h | 240 ++++++++++++++++++++++++ > > drivers/net/ngbe/ngbe_rxtx.c | 16 ++ > > drivers/net/ngbe/ngbe_rxtx.h | 2 + > > 9 files changed, 574 insertions(+) > > create mode 100644 drivers/net/ngbe/ngbe_ptypes.c create mode > 100644 > > drivers/net/ngbe/ngbe_ptypes.h > > > > diff --git a/doc/guides/nics/features/ngbe.ini > > b/doc/guides/nics/features/ngbe.ini > > index 08d5f1b0dc..8b7588184a 100644 > > --- a/doc/guides/nics/features/ngbe.ini > > +++ b/doc/guides/nics/features/ngbe.ini > > @@ -8,6 +8,7 @@ Speed capabilities =3D Y > > Link status =3D Y > > Link status event =3D Y > > Queue start/stop =3D Y > > +Packet type parsing =3D Y >=20 > "Packet type parsing" also requires to support > 'rte_eth_dev_get_supported_ptypes()' & 'rte_eth_dev_set_ptypes()' = APIs. >=20 > Current implementation seems parses the packet type and updates mbuf = field > for it but doesn't support above APIs, can you please add them too? = There is > already 'ngbe_dev_supported_ptypes_get()' function but dev_ops seems = not > set. >=20 Oops.., I forgot it. > <...> >=20 > > +++ b/drivers/net/ngbe/ngbe_ptypes.c > > @@ -0,0 +1,300 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright(c) 2018-2021 Beijing WangXun Technology Co., Ltd. > > + */ > > + > > +#include > > +#include > > + > > +#include "base/ngbe_type.h" > > +#include "ngbe_ptypes.h" > > + > > +/* The ngbe_ptype_lookup is used to convert from the 8-bit ptid in > > +the > > + * hardware to a bit-field that can be used by SW to more easily > > +determine the > > + * packet type. > > + * > > + * Macros are used to shorten the table lines and make this table > > +human > > + * readable. > > + * > > + * We store the PTYPE in the top byte of the bit field - this is = just > > +so that > > + * we can check that the table doesn't have a row missing, as the > > +index into > > + * the table should be the PTYPE. > > + */ > > +#define TPTE(ptid, l2, l3, l4, tun, el2, el3, el4) \ > > + [ptid] =3D (RTE_PTYPE_L2_##l2 | \ > > + RTE_PTYPE_L3_##l3 | \ > > + RTE_PTYPE_L4_##l4 | \ > > + RTE_PTYPE_TUNNEL_##tun | \ > > + RTE_PTYPE_INNER_L2_##el2 | \ > > + RTE_PTYPE_INNER_L3_##el3 | \ > > + RTE_PTYPE_INNER_L4_##el4) > > + > > +#define RTE_PTYPE_L2_NONE 0 > > +#define RTE_PTYPE_L3_NONE 0 > > +#define RTE_PTYPE_L4_NONE 0 > > +#define RTE_PTYPE_TUNNEL_NONE 0 > > +#define RTE_PTYPE_INNER_L2_NONE 0 > > +#define RTE_PTYPE_INNER_L3_NONE 0 > > +#define RTE_PTYPE_INNER_L4_NONE 0 >=20 > Why you are defining new PTYPEs? If these are for driver internal you = can drop > the 'RTE_' prefix. >=20 I just want to use short macros, to make the lookup table readable. So it needs 'RTE_' prefix here, to be compatible with other RTE mbuf = packet types. > <...> >=20 > > + > > +#ifndef RTE_PTYPE_UNKNOWN > > +#define RTE_PTYPE_UNKNOWN 0x00000000 > > +#define RTE_PTYPE_L2_ETHER 0x00000001 > > +#define RTE_PTYPE_L2_ETHER_TIMESYNC 0x00000002 > > +#define RTE_PTYPE_L2_ETHER_ARP 0x00000003 > > +#define RTE_PTYPE_L2_ETHER_LLDP 0x00000004 > > +#define RTE_PTYPE_L2_ETHER_NSH 0x00000005 > > +#define RTE_PTYPE_L2_ETHER_FCOE 0x00000009 > > +#define RTE_PTYPE_L3_IPV4 0x00000010 > > +#define RTE_PTYPE_L3_IPV4_EXT 0x00000030 > > +#define RTE_PTYPE_L3_IPV6 0x00000040 > > +#define RTE_PTYPE_L3_IPV4_EXT_UNKNOWN 0x00000090 > > +#define RTE_PTYPE_L3_IPV6_EXT 0x000000c0 > > +#define RTE_PTYPE_L3_IPV6_EXT_UNKNOWN 0x000000e0 > > +#define RTE_PTYPE_L4_TCP 0x00000100 > > +#define RTE_PTYPE_L4_UDP 0x00000200 > > +#define RTE_PTYPE_L4_FRAG 0x00000300 > > +#define RTE_PTYPE_L4_SCTP 0x00000400 > > +#define RTE_PTYPE_L4_ICMP 0x00000500 > > +#define RTE_PTYPE_L4_NONFRAG 0x00000600 > > +#define RTE_PTYPE_TUNNEL_IP 0x00001000 > > +#define RTE_PTYPE_TUNNEL_GRE 0x00002000 > > +#define RTE_PTYPE_TUNNEL_VXLAN 0x00003000 > > +#define RTE_PTYPE_TUNNEL_NVGRE 0x00004000 > > +#define RTE_PTYPE_TUNNEL_GENEVE 0x00005000 > > +#define RTE_PTYPE_TUNNEL_GRENAT 0x00006000 > > +#define RTE_PTYPE_INNER_L2_ETHER 0x00010000 > > +#define RTE_PTYPE_INNER_L2_ETHER_VLAN 0x00020000 > > +#define RTE_PTYPE_INNER_L3_IPV4 0x00100000 > > +#define RTE_PTYPE_INNER_L3_IPV4_EXT 0x00200000 > > +#define RTE_PTYPE_INNER_L3_IPV6 0x00300000 > > +#define RTE_PTYPE_INNER_L3_IPV4_EXT_UNKNOWN 0x00400000 > > +#define RTE_PTYPE_INNER_L3_IPV6_EXT 0x00500000 > > +#define RTE_PTYPE_INNER_L3_IPV6_EXT_UNKNOWN 0x00600000 > > +#define RTE_PTYPE_INNER_L4_TCP 0x01000000 > > +#define RTE_PTYPE_INNER_L4_UDP 0x02000000 > > +#define RTE_PTYPE_INNER_L4_FRAG 0x03000000 > > +#define RTE_PTYPE_INNER_L4_SCTP 0x04000000 > > +#define RTE_PTYPE_INNER_L4_ICMP 0x05000000 > > +#define RTE_PTYPE_INNER_L4_NONFRAG 0x06000000 > > +#endif /* !RTE_PTYPE_UNKNOWN */ >=20 > These are already defined in the mbuf public header, why there are = defined > again? >=20 These can be removed directly. They were written previously for version = compatibility. > <...> >=20 > > @@ -378,6 +389,10 @@ ngbe_recv_pkts(void *rx_queue, struct rte_mbuf > **rx_pkts, > > rxm->data_len =3D pkt_len; > > rxm->port =3D rxq->port_id; > > > > + pkt_info =3D rte_le_to_cpu_32(rxd.qw0.dw0); > > + rxm->packet_type =3D ngbe_rxd_pkt_info_to_pkt_type(pkt_info, > > + rxq->pkt_type_mask); > > + > > /* > > * Store the mbuf address into the next entry of the array > > * of returned packets. > > @@ -799,6 +814,7 @@ ngbe_dev_rx_queue_setup(struct rte_eth_dev *dev, > > rxq->port_id =3D dev->data->port_id; > > rxq->drop_en =3D rx_conf->rx_drop_en; > > rxq->rx_deferred_start =3D rx_conf->rx_deferred_start; > > + rxq->pkt_type_mask =3D NGBE_PTID_MASK; >=20 > What is the use of the 'pkt_type_mask', it seems it is a fixed value, = why keeping > it per queue?