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

[Bing Zhao]

Hi Thomas,
PSB

> -----Original Message-----
> From: Thomas Monjalon <thomas at monjalon.net>
> Sent: Tuesday, October 13, 2020 5:37 AM
> 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: [dpdk-dev] [PATCH v3 2/6] ethdev: add new attributes to
> hairpin config
> 
> External email: Use caution opening links or attachments
> 
> 
> 08/10/2020 14:05, Bing Zhao:
> >  struct rte_eth_hairpin_conf {
> > -     uint16_t peer_count; /**< The number of peers. */
> > +     uint32_t peer_count:16; /**< The number of peers. */
> 
> Why not keeping uint16_t?

The inside structure has a multiple of 4B, and the peer_count now only takes about 2B. AFAIK, usually, the structure will have an aligned length/offset and there will be some padding between the 2B + (2B pad) + 4B * 32 or 2B + (2B +2B) * 32 + 2B, depending on the compiler.
I changed to bit fields of a u32 due to the two facts:
1. Using the 2B and keep the whole structure aligned. No waste except the reserved bits.
2. Only u32 with bit fields is standard.

> 
> > +     uint32_t tx_explicit:1; /**< Explicit TX flow rule mode. */
> > +     uint32_t manual_bind:1; /**< Manually bind hairpin queues.
> */
> 
> Please describe more the expectations of these bits:
> What is changed at ethdev or PMD level?

In ethdev level, there is almost no change. This attribute will be passed to PMD directly through the function pointer.
In PMD level, these bits should be checked and better to be saved. And the attribute fields should be checked for per queue pair and all the queues, and each queue pair should have the same attributes configured to make the behavior aligned. But it depends on the PMD itself to decide if all the queue pairs between a port pair should have the same attributes, or even all the queues from / to same port of all hairpin port pairs.
If manual_bind is not set, then the PMD will try to bind the queues and enable hairpin during device start stage and there is no need to call the bind / unbind API.
If tx_explicit is set, the application should insert the RX flow rules and TX flow rules seperataly and connect the RX/TX logic connection together.

> What the application is supposed to do?

The application should specify the new two attributes during the queue setup. And also, it could leave it unset (0 by default) to keep the behavior compatible with the previous release.
If manual_bind is set, then it is the application's responsibility to call the bind / unbind API to enable / disable the hairpin between specific ports.
If tx_explicit is set, as described above, the application should maintain the flows logic to make hairpin work as expected, e.g., they can choose metadata (not the only method), in the RX flow, the metadata is set and in the TX flow, it is used for matching. Then even if the headers are changed with NAT action or encap, the hairpin function could work as expected.

> Why choosing one mode or the other?

If the application wants to have the full control of hairpin flows, it could chose the explicit TX flow mode.
If two or more ports involved into the hairpin, it is suggested to use the manual bind.
Please note, the actual supported attributes denpend on the PMD and HW.

> 

Thanks