[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZcHZ6JgacSpGmyDQ@mev-dev>
Date: Tue, 6 Feb 2024 08:04:08 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Michael Chan <michael.chan@...adcom.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, andrew.gospodarek@...adcom.com,
pavan.chebbi@...adcom.com
Subject: Re: [PATCH net-next 02/13] bnxt_en: Add ethtool -N support for ether
filters.
On Mon, Feb 05, 2024 at 02:31:51PM -0800, Michael Chan wrote:
> Add ETHTOOL_SRXCLSRLINS and ETHTOOL_SRXCLSRLDEL support for inserting
> and deleting L2 ether filter rules. Destination MAC address and
> optional VLAN are supported for each filter entry. This is currently
> only supported on older BCM573XX and BCM574XX chips only.
>
> Reviewed-by: Pavan Chebbi <pavan.chebbi@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 34 +++++++++
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 3 +
> .../net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 69 ++++++++++++++++++-
> 3 files changed, 103 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 91ecf514b924..3cc3504181c7 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -5519,6 +5519,40 @@ static struct bnxt_l2_filter *bnxt_alloc_l2_filter(struct bnxt *bp,
> return fltr;
> }
>
> +struct bnxt_l2_filter *bnxt_alloc_new_l2_filter(struct bnxt *bp,
> + struct bnxt_l2_key *key,
> + u16 flags)
> +{
> + struct bnxt_l2_filter *fltr;
> + u32 idx;
> + int rc;
> +
> + idx = jhash2(&key->filter_key, BNXT_L2_KEY_SIZE, bp->hash_seed) &
> + BNXT_L2_FLTR_HASH_MASK;
> + spin_lock_bh(&bp->ntp_fltr_lock);
> + fltr = __bnxt_lookup_l2_filter(bp, key, idx);
> + if (fltr) {
> + fltr = ERR_PTR(-EEXIST);
> + goto l2_filter_exit;
> + }
> + fltr = kzalloc(sizeof(*fltr), GFP_ATOMIC);
> + if (!fltr) {
> + fltr = ERR_PTR(-ENOMEM);
> + goto l2_filter_exit;
> + }
> + fltr->base.flags = flags;
> + rc = bnxt_init_l2_filter(bp, fltr, key, idx);
> + if (rc) {
> + spin_unlock_bh(&bp->ntp_fltr_lock);
Why filter needs to be deleted without lock? If you can change the order
it looks more natural:
+if (rc) {
+ fltr = ERR_PTR(rc);
+ goto l2_filter_del;
+}
> + bnxt_del_l2_filter(bp, fltr);
> + return ERR_PTR(rc);
> + }
> +
> +l2_filter_exit:
> + spin_unlock_bh(&bp->ntp_fltr_lock);
> + return fltr;
> +}
> +
> static u16 bnxt_vf_target_id(struct bnxt_pf_info *pf, u16 vf_idx)
> {
> #ifdef CONFIG_BNXT_SRIOV
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index 4bd1cf01d99e..21721b8748bc 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -2646,6 +2646,9 @@ int bnxt_hwrm_func_drv_rgtr(struct bnxt *bp, unsigned long *bmap,
> int bmap_size, bool async_only);
> int bnxt_hwrm_func_drv_unrgtr(struct bnxt *bp);
> void bnxt_del_l2_filter(struct bnxt *bp, struct bnxt_l2_filter *fltr);
> +struct bnxt_l2_filter *bnxt_alloc_new_l2_filter(struct bnxt *bp,
> + struct bnxt_l2_key *key,
> + u16 flags);
> int bnxt_hwrm_l2_filter_free(struct bnxt *bp, struct bnxt_l2_filter *fltr);
> int bnxt_hwrm_l2_filter_alloc(struct bnxt *bp, struct bnxt_l2_filter *fltr);
> int bnxt_hwrm_cfa_ntuple_filter_free(struct bnxt *bp,
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> index 1c8610386404..2d8e847e8fdd 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
> @@ -1152,6 +1152,58 @@ static int bnxt_grxclsrule(struct bnxt *bp, struct ethtool_rxnfc *cmd)
> return rc;
> }
>
> +static int bnxt_add_l2_cls_rule(struct bnxt *bp,
> + struct ethtool_rx_flow_spec *fs)
> +{
> + u32 ring = ethtool_get_flow_spec_ring(fs->ring_cookie);
> + u8 vf = ethtool_get_flow_spec_ring_vf(fs->ring_cookie);
> + struct ethhdr *h_ether = &fs->h_u.ether_spec;
> + struct ethhdr *m_ether = &fs->m_u.ether_spec;
> + struct bnxt_l2_filter *fltr;
> + struct bnxt_l2_key key;
> + u16 vnic_id;
> + u8 flags;
> + int rc;
> +
> + if (BNXT_CHIP_P5_PLUS(bp))
> + return -EOPNOTSUPP;
> +
> + if (!is_broadcast_ether_addr(m_ether->h_dest))
> + return -EINVAL;
> + ether_addr_copy(key.dst_mac_addr, h_ether->h_dest);
> + key.vlan = 0;
> + if (fs->flow_type & FLOW_EXT) {
> + struct ethtool_flow_ext *m_ext = &fs->m_ext;
> + struct ethtool_flow_ext *h_ext = &fs->h_ext;
> +
> + if (m_ext->vlan_tci != htons(0xfff) || !h_ext->vlan_tci)
> + return -EINVAL;
> + key.vlan = ntohs(h_ext->vlan_tci);
> + }
> +
> + if (vf) {
> + flags = BNXT_ACT_FUNC_DST;
> + vnic_id = 0xffff;
> + vf--;
> + } else {
> + flags = BNXT_ACT_RING_DST;
> + vnic_id = bp->vnic_info[ring + 1].fw_vnic_id;
> + }
> + fltr = bnxt_alloc_new_l2_filter(bp, &key, flags);
> + if (IS_ERR(fltr))
> + return PTR_ERR(fltr);
> +
> + fltr->base.fw_vnic_id = vnic_id;
> + fltr->base.rxq = ring;
> + fltr->base.vf_idx = vf;
> + rc = bnxt_hwrm_l2_filter_alloc(bp, fltr);
> + if (rc)
> + bnxt_del_l2_filter(bp, fltr);
> + else
> + fs->location = fltr->base.sw_id;
> + return rc;
> +}
> +
> #define IPV4_ALL_MASK ((__force __be32)~0)
> #define L4_PORT_ALL_MASK ((__force __be16)~0)
>
> @@ -1335,7 +1387,7 @@ static int bnxt_srxclsrlins(struct bnxt *bp, struct ethtool_rxnfc *cmd)
> return -EINVAL;
> flow_type &= ~FLOW_EXT;
> if (flow_type == ETHER_FLOW)
> - rc = -EOPNOTSUPP;
> + rc = bnxt_add_l2_cls_rule(bp, fs);
> else
> rc = bnxt_add_ntuple_cls_rule(bp, fs);
> return rc;
> @@ -1346,11 +1398,22 @@ static int bnxt_srxclsrldel(struct bnxt *bp, struct ethtool_rxnfc *cmd)
> struct ethtool_rx_flow_spec *fs = &cmd->fs;
> struct bnxt_filter_base *fltr_base;
> struct bnxt_ntuple_filter *fltr;
> + u32 id = fs->location;
>
> rcu_read_lock();
> + fltr_base = bnxt_get_one_fltr_rcu(bp, bp->l2_fltr_hash_tbl,
> + BNXT_L2_FLTR_HASH_SIZE, id);
> + if (fltr_base) {
> + struct bnxt_l2_filter *l2_fltr;
> +
> + l2_fltr = container_of(fltr_base, struct bnxt_l2_filter, base);
> + rcu_read_unlock();
> + bnxt_hwrm_l2_filter_free(bp, l2_fltr);
> + bnxt_del_l2_filter(bp, l2_fltr);
> + return 0;
> + }
> fltr_base = bnxt_get_one_fltr_rcu(bp, bp->ntp_fltr_hash_tbl,
> - BNXT_NTP_FLTR_HASH_SIZE,
> - fs->location);
> + BNXT_NTP_FLTR_HASH_SIZE, id);
> if (!fltr_base) {
> rcu_read_unlock();
> return -ENOENT;
> --
> 2.30.1
>
Powered by blists - more mailing lists