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 <jiawe...@trustnetic.com> > > --- > > 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 = Y > > Link status = Y > > Link status event = Y > > Queue start/stop = Y > > +Packet type parsing = Y > > "Packet type parsing" also requires to support > 'rte_eth_dev_get_supported_ptypes()' & 'rte_eth_dev_set_ptypes()' APIs. > > 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. >
Oops.., I forgot it. > <...> > > > +++ 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 <rte_mbuf.h> > > +#include <rte_memory.h> > > + > > +#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] = (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 > > Why you are defining new PTYPEs? If these are for driver internal you can drop > the 'RTE_' prefix. > 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. > <...> > > > + > > +#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 */ > > These are already defined in the mbuf public header, why there are defined > again? > These can be removed directly. They were written previously for version compatibility. > <...> > > > @@ -378,6 +389,10 @@ ngbe_recv_pkts(void *rx_queue, struct rte_mbuf > **rx_pkts, > > rxm->data_len = pkt_len; > > rxm->port = rxq->port_id; > > > > + pkt_info = rte_le_to_cpu_32(rxd.qw0.dw0); > > + rxm->packet_type = 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 = dev->data->port_id; > > rxq->drop_en = rx_conf->rx_drop_en; > > rxq->rx_deferred_start = rx_conf->rx_deferred_start; > > + rxq->pkt_type_mask = NGBE_PTID_MASK; > > What is the use of the 'pkt_type_mask', it seems it is a fixed value, why > keeping > it per queue?