[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20231225171125.GJ5962@kernel.org>
Date: Mon, 25 Dec 2023 17:11:25 +0000
From: Simon Horman <horms@...nel.org>
To: Michael Chan <michael.chan@...adcom.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, pavan.chebbi@...adcom.com,
andrew.gospodarek@...adcom.com
Subject: Re: [PATCH net-next v2 08/13] bnxt_en: Refactor filter insertion
logic in bnxt_rx_flow_steer().
On Fri, Dec 22, 2023 at 08:22:05PM -0800, Michael Chan wrote:
> Add a new function bnxt_insert_ntp_filter() to insert the ntuple filter
> into the hash table and other basic setup. We'll use this function
> to insert a user defined filter from ethtool.
>
> Also, export bnxt_lookup_ntp_filter_from_idx() and bnxt_get_ntp_filter_idx()
> for similar purposes. All ntuple related functions are now no longer
> compiled only for CONFIG_RFS_ACCEL
>
> Reviewed-by: Vasundhara Volam <vasundhara-v.volam@...adcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@...adcom.com>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
...
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
...
> @@ -13991,33 +13995,20 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb,
> }
> rcu_read_unlock();
>
> - spin_lock_bh(&bp->ntp_fltr_lock);
> - bit_id = bitmap_find_free_region(bp->ntp_fltr_bmap, BNXT_MAX_FLTR, 0);
> - if (bit_id < 0) {
> - spin_unlock_bh(&bp->ntp_fltr_lock);
> - rc = -ENOMEM;
> - goto err_free;
> - }
> -
> - new_fltr->base.sw_id = (u16)bit_id;
> new_fltr->flow_id = flow_id;
> new_fltr->base.rxq = rxq_index;
> - new_fltr->base.type = BNXT_FLTR_TYPE_NTUPLE;
> - new_fltr->base.flags = BNXT_ACT_RING_DST;
> - hlist_add_head_rcu(&new_fltr->base.hash, head);
> - set_bit(BNXT_FLTR_INSERTED, &new_fltr->base.state);
> - bp->ntp_fltr_count++;
> - spin_unlock_bh(&bp->ntp_fltr_lock);
> -
> - bnxt_queue_sp_work(bp, BNXT_RX_NTP_FLTR_SP_EVENT);
> -
> - return new_fltr->base.sw_id;
> + rc = bnxt_insert_ntp_filter(bp, new_fltr, idx);
> + if (!rc) {
> + bnxt_queue_sp_work(bp, BNXT_RX_NTP_FLTR_SP_EVENT);
> + return new_fltr->base.sw_id;
> + }
Hi Michael,
FIWIW, I think the following would be a more idomatic flow.
(Completely untested!)
rc = bnxt_insert_ntp_filter(bp, new_fltr, idx);
if (rc)
goto err_free;
bnxt_queue_sp_work(bp, BNXT_RX_NTP_FLTR_SP_EVENT);
return new_fltr->base.sw_id;
>
> err_free:
> bnxt_del_l2_filter(bp, l2_fltr);
> kfree(new_fltr);
> return rc;
> }
> +#endif
>
> static void bnxt_cfg_ntp_filters(struct bnxt *bp)
> {
...
Powered by blists - more mailing lists