Re: [PATCH] app/testpmd: add ipv6 extension header parse

[Thomas Monjalon <thomas@monjalon.net>]

14/01/2025 03:05, Jie Hai:
> Hi, Stephen Hemminger,
> 
> On 2025/1/9 1:02, Stephen Hemminger wrote:
> > On Wed, 8 Jan 2025 10:46:32 +0800
> > Jie Hai <haijie1@huawei.com> wrote:
> > 
> >> From: Jie Hai <haijie1@huawei.com>
> >> To: <dev@dpdk.org>, <thomas@monjalon.net>, <ferruh.yigit@amd.com>, Aman Singh  <aman.deep.singh@intel.com>
> >> CC: <lihuisong@huawei.com>, <fengcengwen@huawei.com>, <haijie1@huawei.com>,  <huangdengdui@huawei.com>
> >> Subject: [PATCH] app/testpmd: add ipv6 extension header parse
> >> Date: Wed, 8 Jan 2025 10:46:32 +0800
> >> X-Mailer: git-send-email 2.22.0
> >>
> >> This patch support parse ipv6 extension header, and
> >> support TSO for ipv6tcp packets with extension header.
> >>
> >> Signed-off-by: Jie Hai <haijie1@huawei.com>
> >> ---
> >>   app/test-pmd/csumonly.c | 47 ++++++++++++++++++++++++++++++++++++++++-
> >>   1 file changed, 46 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> >> index 2246c22e8e56..a7b11490fe27 100644
> >> --- a/app/test-pmd/csumonly.c
> >> +++ b/app/test-pmd/csumonly.c
> >> @@ -124,14 +124,59 @@ parse_ipv4(struct rte_ipv4_hdr *ipv4_hdr, struct testpmd_offload_info *info)
> >>   		info->l4_len = 0;
> >>   }
> >>   
> >> +static uint16_t
> >> +parse_ipv6_ext(struct rte_ipv6_hdr *ipv6_hdr, uint32_t *off)
> >> +{
> >> +	struct ext_hdr {
> >> +		uint8_t next_hdr;
> >> +		uint8_t len;
> >> +	};
> >> +	struct ext_hdr *xh;
> >> +	uint16_t proto;
> >> +	char *xh_fst;
> >> +	uint16_t i;
> >> +
> >> +	proto = ipv6_hdr->proto;
> >> +	xh_fst = (char *)ipv6_hdr + sizeof(*ipv6_hdr);
> >> +#define MAX_EXT_HDRS 9
> >> +	for (i = 0; i < MAX_EXT_HDRS; i++) {
> >> +		switch (proto) {
> >> +		case IPPROTO_HOPOPTS:
> >> +		case IPPROTO_ROUTING:
> >> +		case IPPROTO_DSTOPTS:
> >> +			xh = (struct ext_hdr *)(xh_fst + *off);
> >> +			*off += (xh->len + 1) * 8;
> >> +			proto = xh->next_hdr;
> >> +			break;
> >> +		case IPPROTO_AH:
> >> +			xh = (struct ext_hdr *)(xh_fst + *off);
> >> +			*off += (xh->len + 2) * 4;
> >> +			proto = xh->next_hdr;
> >> +			break;
> >> +		case IPPROTO_FRAGMENT:
> >> +			xh = (struct ext_hdr *)(xh_fst + *off);
> >> +			*off += 8;
> >> +			proto = xh->next_hdr;
> >> +			return proto; /* this is always the last ext hdr */
> >> +		case IPPROTO_NONE:
> >> +			return proto;
> >> +		default:
> >> +			return proto;
> >> +		}
> >> +	}
> >> +	return proto;
> >> +}
> >> +
> > 
> > Why copy/paste of rte_net_skip_ip6_ext, why not use that?
> > Having two copies of same codes means that bugs need to be fixed in two places later.
> > .
> Thanks for your review.
> 
> rte_net_skip_ip6_ext uses mbuf as a parameter, but its upper-layer
> function does not pass this parameter, and it is difficult to deduce the
> mbuf address. It would also be strange to pass only the mbuf address and
> not use it.
> 
> My idea is to replace the packet identification in csum fwd with
> 'rte_net_get_ptype()'. In this case, 'rte_net_get_ptype' needs to be 
> updated to adapt to the packet types supported by csum fwd. This may
> affect the ptype of packets identified by the current software.
> I'm not sure how big the impact is, which is why I didn't use it.
> 
> Maybe I can send it out later for review.

The only thing I'm sure is that testpmd is a test application,
so it should reuse the functions from the libs.
It should not reinvent new functions for network parsing.

If you need to add a new function to the library, please do it.
You can probably move the content of rte_net_skip_ip6_ext()
in a new function in the file lib/net/rte_ip6.h.
Something similar to rte_ipv6_get_next_ext().