Hi Thomas,
All comments are addressed and thanks a lot for the reviewing.
> -----Original Message-----
> From: Thomas Monjalon <tho...@monjalon.net>
> Sent: Thursday, October 15, 2020 6:46 PM
> To: Bing Zhao <bi...@nvidia.com>
> Cc: Ori Kam <or...@nvidia.com>; ferruh.yi...@intel.com;
> arybche...@solarflare.com; m...@ashroe.eu; nhor...@tuxdriver.com;
> bernard.iremon...@intel.com; beilei.x...@intel.com;
> wenzhuo...@intel.com; dev@dpdk.org
> Subject: Re: [PATCH v5 2/5] ethdev: add new attributes to hairpin
> config
>
> External email: Use caution opening links or attachments
>
>
> 15/10/2020 07:35, Bing Zhao:
> > To support two ports hairpin mode and keep the backward
> compatibility
> > for the application, two new attribute members of the hairpin
> queue
> > configuration structure will be added.
> >
> > `tx_explicit` means if the application itself will insert the TX
> part
> > flow rules. If not set, PMD will insert the rules implicitly.
> > `manual_bind` means if the hairpin TX queue and peer RX queue will
> be
> > bound automatically during the device start stage.
> >
> > Different TX and RX queue pairs could have different values, but
> it is
> > highly recommended that all paired queues between one egress and
> its
> > peer ingress ports have the same values, in order not to bring any
> > chaos to the system. The actual support of these attribute
> parameters
> > will be checked and decided by the PMD drivers.
> >
> > In the single port hairpin, if both are zero without any setting,
> the
> > behavior will remain the same as before. It means that no bind API
> > needs to be called and no TX flow rules need to be inserted
> manually
> > by the application.
> >
> > Signed-off-by: Bing Zhao <bi...@nvidia.com>
> > Acked-by: Ori Kam <or...@nvidia.com>
> > ---
> > v4: squash document update and more info for the two new
> attributes
> > v2: optimize the structure and remove unused macros
> > ---
>
> Acked-by: Thomas Monjalon <tho...@monjalon.net>
>
> Minor comments below.
>
> > struct rte_eth_hairpin_conf {
> > - uint16_t peer_count; /**< The number of peers. */
> > + uint32_t peer_count:16; /**< The number of peers. */
> > +
> > + /**
> > + * Explicit TX flow rule mode. One hairpin pair of queues
> should have
> > + * the same attribute. The actual support depends on the PMD.
>
> The second sentence should be on a separate line.
>
> About the third sentence, implementation is always PMD-specific.
> PMD will reject the not supported conf, as usual.
> I think this comment is not needed in the API description.
>
> > + *
> > + * - When set, the user should be responsible for inserting
> the hairpin
> > + * TX part flows and removing them.
> > + * - When clear, the PMD will try to handle the TX part of
> the flows,
> > + * e.g., by splitting one flow into two parts.
> > + */
> > + uint32_t tx_explicit:1;
> > +
> > + /**
> > + * Manually bind hairpin queues. One hairpin pair of queues
> should have
> > + * the same attribute. The actual support depends on the PMD.
>
> Same here
>
> > + *
> > + * - When set, to enable hairpin, the user should call the
> hairpin bind
> > + * API after all the queues are set up properly and the
> ports are
> > + * started. Also, the hairpin unbind API should be called
> accordingly
> > + * before stopping a port that with hairpin configured.
> > + * - When clear, the PMD will try to enable the hairpin with
> the queues
> > + * configured automatically during port start.
> > + */
> > + uint32_t manual_bind:1;
> > + uint32_t reserved:14; /**< Reserved bits. */
> > struct rte_eth_hairpin_peer peers[RTE_ETH_MAX_HAIRPIN_PEERS];
> > };
>
>
