[dpdk-dev] [PATCH v5 2/5] ethdev: add new attributes to hairpin config

[Bing Zhao]

Hi Thomas,

All comments are addressed and thanks a lot for the reviewing.

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Thursday, October 15, 2020 6:46 PM
> To: Bing Zhao <bingz at nvidia.com>
> Cc: Ori Kam <orika at nvidia.com>; ferruh.yigit at intel.com;
> arybchenko at solarflare.com; mdr at ashroe.eu; nhorman at tuxdriver.com;
> bernard.iremonger at intel.com; beilei.xing at intel.com;
> wenzhuo.lu at intel.com; dev at 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 <bingz at nvidia.com>
> > Acked-by: Ori Kam <orika at 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 <thomas at 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];
> > };
> 
>