Re: [dpdk-dev] [PATCH v5 1/5] ethdev: add hairpin bind and unbind APIs

[Bing Zhao <bingz@nvidia.com>]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Thursday, October 15, 2020 6:34 PM
> To: Bing Zhao <bingz@nvidia.com>
> Cc: Ori Kam <orika@nvidia.com>; ferruh.yigit@intel.com;
> arybchenko@solarflare.com; mdr@ashroe.eu; nhorman@tuxdriver.com;
> bernard.iremonger@intel.com; beilei.xing@intel.com;
> wenzhuo.lu@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v5 1/5] ethdev: add hairpin bind and unbind APIs
> 
> External email: Use caution opening links or attachments
> 
> 
> 15/10/2020 07:35, Bing Zhao:
> > v5:
> >   * Change EINVAL to ENODEV
> >   * add newline character in the end of log line
> >   * descriptions update
> 
> It looks good.
> More minor coding style comments below. With those,
> Acked-by: Thomas Monjalon <thomas@monjalon.net>

Thanks for the review and comments

> 
> > +     if (ret)
> 
> Coding style recommends explicit comparison with == or !=

Done

> 
> > +             RTE_ETHDEV_LOG(ERR, "Failed to bind hairpin TX %d "
> > +                            "to RX %d (%d - all ports)\n",
> tx_port,
> > +                            rx_port, RTE_MAX_ETHPORTS);
> 
> It is preferred not splitting the log lines, or maybe only after a
> format specifier, so it can be grepped.
> Here the space after %d would be better on the next line.
> 
> In general Rx/Tx is preferred over the full capital RX/TX version.

Done

> 
> Thanks
> 

BR. Bing