[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <244138b2-a90e-404d-946f-9ce25c6155e1@redhat.com>
Date: Thu, 8 May 2025 16:00:05 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Roger Quadros <rogerq@...nel.org>,
Siddharth Vadapalli <s-vadapalli@...com>, Andrew Lunn
<andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
Russell King <linux@...linux.org.uk>, danishanwar@...com
Cc: srk@...com, linux-omap@...r.kernel.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v2 8/9] net: ethernet: ti: am65-cpsw: add network
flow classification support
On 5/5/25 6:26 PM, Roger Quadros wrote:
[...]
> +/* validate the rxnfc rule and convert it to policer config */
> +static int am65_cpsw_rxnfc_validate(struct am65_cpsw_port *port,
> + struct ethtool_rxnfc *rxnfc,
> + struct cpsw_ale_policer_cfg *cfg)
> +{
> + struct ethtool_rx_flow_spec *fs = &rxnfc->fs;
> + int flow_type = AM65_CPSW_FLOW_TYPE(fs->flow_type);
> + struct ethhdr *eth_mask;
(Minor nit only mentioned because of more relevant comments on previous
patch) Please respect the reverse christmas tree order above.
> +
> + memset(cfg, 0, sizeof(*cfg));
> +
> + if (flow_type & FLOW_RSS)
> + return -EINVAL;
> +
> + if (fs->location == RX_CLS_LOC_ANY ||
> + fs->location >= port->rxnfc_max)
> + return -EINVAL;
> +
> + if (fs->ring_cookie == RX_CLS_FLOW_DISC)
> + cfg->drop = true;
> + else if (fs->ring_cookie > AM65_CPSW_MAX_QUEUES)
> + return -EINVAL;
> +
> + cfg->port_id = port->port_id;
> + cfg->thread_id = fs->ring_cookie;
> +
> + switch (flow_type) {
> + case ETHER_FLOW:
> + eth_mask = &fs->m_u.ether_spec;
> +
> + /* etherType matching is supported by h/w but not yet here */
> + if (eth_mask->h_proto)
> + return -EINVAL;
> +
> + /* Only support source matching addresses by full mask */
> + if (is_broadcast_ether_addr(eth_mask->h_source)) {
> + cfg->match_flags |= CPSW_ALE_POLICER_MATCH_MACSRC;
> + ether_addr_copy(cfg->src_addr,
> + fs->h_u.ether_spec.h_source);
> + }
> +
> + /* Only support destination matching addresses by full mask */
> + if (is_broadcast_ether_addr(eth_mask->h_dest)) {
> + cfg->match_flags |= CPSW_ALE_POLICER_MATCH_MACDST;
> + ether_addr_copy(cfg->dst_addr,
> + fs->h_u.ether_spec.h_dest);
> + }
> +
> + if ((fs->flow_type & FLOW_EXT) && fs->m_ext.vlan_tci) {
> + /* Don't yet support vlan ethertype */
> + if (fs->m_ext.vlan_etype)
> + return -EINVAL;
> +
> + if (fs->m_ext.vlan_tci != VLAN_TCI_FULL_MASK)
> + return -EINVAL;
> +
> + cfg->vid = FIELD_GET(VLAN_VID_MASK,
> + ntohs(fs->h_ext.vlan_tci));
> + cfg->vlan_prio = FIELD_GET(VLAN_PRIO_MASK,
> + ntohs(fs->h_ext.vlan_tci));
> + cfg->match_flags |= CPSW_ALE_POLICER_MATCH_OVLAN;
> + }
> +
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int am65_cpsw_policer_find_match(struct am65_cpsw_port *port,
> + struct cpsw_ale_policer_cfg *cfg)
> +{
> + struct am65_cpsw_rxnfc_rule *rule;
> + int loc = -EINVAL;
> +
> + mutex_lock(&port->rxnfc_lock);
> + list_for_each_entry(rule, &port->rxnfc_rules, list) {
> + if (!memcmp(&rule->cfg, cfg, sizeof(*cfg))) {
> + loc = rule->location;
> + break;
> + }
> + }
> +
> + mutex_unlock(&port->rxnfc_lock);
> +
> + return loc;
> +}
> +
> +static int am65_cpsw_rxnfc_add_rule(struct am65_cpsw_port *port,
> + struct ethtool_rxnfc *rxnfc)
> +{
> + struct ethtool_rx_flow_spec *fs = &rxnfc->fs;
> + struct am65_cpsw_rxnfc_rule *rule;
> + struct cpsw_ale_policer_cfg cfg;
> + int loc, ret;
> +
> + if (am65_cpsw_rxnfc_validate(port, rxnfc, &cfg))
> + return -EINVAL;
> +
> + /* need to check if similar rule is already present at another location,
> + * if yes error out
> + */
> + loc = am65_cpsw_policer_find_match(port, &cfg);
> + if (loc >= 0 && loc != fs->location) {
> + netdev_info(port->ndev,
> + "rule already exists in location %d. not adding\n",
> + loc);
> + return -EINVAL;
> + }
> +
> + /* delete exisiting rule */
> + if (loc >= 0) {
> + mutex_lock(&port->rxnfc_lock);
The rxnfc_lock mutex is released and re-aquired after the previous
lookup. Con some other thread delete the matching rule in-between and
add another one at a different location?
/P
Powered by blists - more mailing lists