Re: [PATCH 10/20] net/cnxk: added Rx metadata negotiate operation

[Nithin Kumar Dabilpuram <ndabilpuram@marvell.com>]

Please see inline.

Thanks
Nithin

On 2/17/22 7:03 PM, Jerin Jacob wrote:
> On Mon, Feb 7, 2022 at 1:01 PM Nithin Dabilpuram
> <ndabilpuram@marvell.com> wrote:
>>
>> From: Satha Rao <skoteshwar@marvell.com>
>>
>> Added rx_metadata_negotiate api to enable mark update RX offload.
>> Removed software logic to enable/disable mark update inside flow
>> create/destroy apis.
> 
> APIs

Ack.
> 
>> Signed-off-by: Satha Rao <skoteshwar@marvell.com>
>> ---
> 
>> +static int
>> +cn10k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
>> +{
>> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>> +
>> +       *features &=
>> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
>> +
>> +       if (*features) {
>> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = true;
>> +       } else {
>> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = false;
>> +       }
>> +
>> +       cn10k_eth_set_rx_function(eth_dev);
>> +
>> +       return 0;
>> +}
> 
> See below.
> 
>>
>> +static int
>> +cn9k_nix_rx_metadata_negotiate(struct rte_eth_dev *eth_dev, uint64_t *features)
>> +{
>> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>> +
>> +       *features &=
>> +               (RTE_ETH_RX_METADATA_USER_FLAG | RTE_ETH_RX_METADATA_USER_MARK);
>> +
>> +       if (*features) {
>> +               dev->rx_offload_flags |= NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = true;
>> +       } else {
>> +               dev->rx_offload_flags &= ~NIX_RX_OFFLOAD_MARK_UPDATE_F;
>> +               dev->rx_mark_update = false;
>> +       }
>> +
>> +       cn9k_eth_set_rx_function(eth_dev);
>> +
>> +       return 0;
>> +}
>> +
> 
> The above two functions are duplicates, Please pass the function
> pointer and make it as one function
> for common code.

Though the two functions are same, the fast path offload flags used 
within them are defined per platform so that in future CN9K and CN10K
can have different set of fast path offloads though the two hash defines
are same now. They are also in mutually exclusive files.