RE: [PATCH] net/af_packet: add timestamp offloading support

["Stefan Lässer" <stefan.laesser@omicronenergy.com>]

> > +uint64_t af_packet_timestamp_dynflag; int
> > +af_packet_timestamp_dynfield_offset = -1;
> 
> No need to expose these publicly, they should be static.
> This also means that you can remove the af_packet_ prefix.
> >  eth_dev_start(struct rte_eth_dev *dev)  {
> >  	struct pmd_internals *internals = dev->data->dev_private;
> > -	uint16_t i;
> > +
> > +	if (internals->timestamp_offloading) {
> > +		/* Register mbuf field and flag for Rx timestamp */
> > +		int rc =
> >
> rte_mbuf_dyn_rx_timestamp_register(&af_packet_timestamp_dynfield_offs
> e
> > t,
> > +				&af_packet_timestamp_dynflag);
> > +		if (rc) {
> > +			PMD_LOG(ERR, "Cannot register mbuf field/flag for
> > timestamp");
> > +			return rc;
> > +		}
> > +	}
> >
> >  	dev->data->dev_link.link_status = RTE_ETH_LINK_UP;
> > +
> > +	uint16_t i;
> 
> No need to move this down here. Please leave it where it was.
> 
> Alternatively, minimize its scope by moving it inside the for loop:
> -	for (i = 0; i < internals->nb_queues; i++) {
> +	for (uint16_t i = 0; i < internals->nb_queues; i++) {
> 
> 
> With the two variables made local,
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>

Thank you very much for your feedback! I have submitted a patch including your recommendations.

I will start with the other patches now.