Re: [PATCH v3 1/2] examples/l3fwd: common packet group functionality

[Thomas Monjalon <thomas@monjalon.net>]

23/06/2022 11:38, Rahul Bhansali:
> +#ifndef _PORT_GROUP_H_
> +#define _PORT_GROUP_H_

No need of underscores at begin and end.

> +
> +#include "pkt_group.h"
> +
> +/*
> + * Group consecutive packets with the same destination port in bursts of 4.
> + * Suppose we have array of destination ports:
> + * dst_port[] = {a, b, c, d,, e, ... }
> + * dp1 should contain: <a, b, c, d>, dp2: <b, c, d, e>.
> + * We doing 4 comparisons at once and the result is 4 bit mask.
> + * This mask is used as an index into prebuild array of pnum values.
> + */

This explanation is not clear to me.

> +static inline uint16_t *
> +port_groupx4(uint16_t pn[FWDSTEP + 1], uint16_t *lp,

array parameter is not standard, you should make it a simple pointer

> +	     __vector unsigned short dp1,
> +	     __vector unsigned short dp2)


longer parameter names would help

[...]
> --- a/examples/l3fwd/Makefile
> +++ b/examples/l3fwd/Makefile
> +INCLUDES =-I../common
>  PC_FILE := $(shell $(PKGCONF) --path libdpdk 2>/dev/null)
>  CFLAGS += -O3 $(shell $(PKGCONF) --cflags libdpdk)
>  # Added for 'rte_eth_link_to_str()'
> @@ -38,10 +39,10 @@ endif
>  endif
> 
>  build/$(APP)-shared: $(SRCS-y) Makefile $(PC_FILE) | build
> -	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
> +	$(CC) $(CFLAGS) $(SRCS-y) $(INCLUDES) -o $@ $(LDFLAGS) $(LDFLAGS_SHARED)
> 
>  build/$(APP)-static: $(SRCS-y) Makefile $(PC_FILE) | build
> -	$(CC) $(CFLAGS) $(SRCS-y) -o $@ $(LDFLAGS) $(LDFLAGS_STATIC)
> +	$(CC) $(CFLAGS) $(SRCS-y) $(INCLUDES) -o $@ $(LDFLAGS) $(LDFLAGS_STATIC)

No need to introduce INCLUDES, you can expand CFLAGS.

I will fix this last one while pulling.
Please work on better names and explanations for an EAL integration.