* [PATCH] idpf: fix issue with ethtool -n command display
@ 2025-09-25 15:24 Sreedevi Joshi
2025-09-25 15:27 ` Joshi, Sreedevi
0 siblings, 1 reply; 2+ messages in thread
From: Sreedevi Joshi @ 2025-09-25 15:24 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, Erik Gabriel Carrillo, Sreedevi Joshi, Przemek Kitszel,
Aleksandr Loktionov
From: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
When ethtool -n is executed on an interface to display the flow steering
rules, "rxclass: Unknown flow type" error is generated.
The flow steering list maintained in the driver currently stores only the
location and q_index but other fields of the ethtool_rx_flow_spec are not
stored. This may be enough for the virtchnl command to delete the entry.
However, when the ethtool -n command is used to query the flow steering
rules, the ethtool_rx_flow_spec returned is not complete causing the
error below.
Resolve this by storing the flow spec (fsp) when rules are added and
returning the complete flow spec when rules are queried.
Also, change the return value from EINVAL to ENOENT when flow steering
entry is not found during query by location or when deleting an entry.
Add logic to detect and reject duplicate filter entries at the same
location.
Example:
Before the fix:
ethtool -n eth1
2 RX rings available
Total 2 rules
rxclass: Unknown flow type
rxclass: Unknown flow type
After the fix:
ethtool -n eth1
2 RX rings available
Total 2 rules
Filter: 0
Rule Type: TCP over IPv4
Src IP addr: 10.0.0.1 mask: 0.0.0.0
Dest IP addr: 0.0.0.0 mask: 255.255.255.255
TOS: 0x0 mask: 0xff
Src port: 0 mask: 0xffff
Dest port: 0 mask: 0xffff
Action: Direct to queue 0
Filter: 1
Rule Type: UDP over IPv4
Src IP addr: 10.0.0.1 mask: 0.0.0.0
Dest IP addr: 0.0.0.0 mask: 255.255.255.255
TOS: 0x0 mask: 0xff
Src port: 0 mask: 0xffff
Dest port: 0 mask: 0xffff
Action: Direct to queue 0
Fixes: ada3e24b84a0 ("idpf: add flow steering support")
Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
Co-developed-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
---
drivers/net/ethernet/intel/idpf/idpf.h | 3 +-
.../net/ethernet/intel/idpf/idpf_ethtool.c | 57 ++++++++++++-------
2 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
index 4f4cf21e3c46..ec759bc5b3ce 100644
--- a/drivers/net/ethernet/intel/idpf/idpf.h
+++ b/drivers/net/ethernet/intel/idpf/idpf.h
@@ -269,8 +269,7 @@ struct idpf_port_stats {
struct idpf_fsteer_fltr {
struct list_head list;
- u32 loc;
- u32 q_index;
+ struct ethtool_rx_flow_spec fs;
};
/**
diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
index 1352f18b60b0..6a39cc1feeb5 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
@@ -38,11 +38,13 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
cmd->data = idpf_fsteer_max_rules(vport);
break;
case ETHTOOL_GRXCLSRULE:
- err = -EINVAL;
+ err = -ENOENT;
spin_lock_bh(&vport_config->flow_steer_list_lock);
list_for_each_entry(f, &user_config->flow_steer_list, list)
- if (f->loc == cmd->fs.location) {
- cmd->fs.ring_cookie = f->q_index;
+ if (f->fs.location == cmd->fs.location) {
+ /* Avoid infoleak from padding: zero first, then assign fields */
+ memset(&cmd->fs, 0, sizeof(cmd->fs));
+ cmd->fs = f->fs;
err = 0;
break;
}
@@ -56,7 +58,7 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
err = -EMSGSIZE;
break;
}
- rule_locs[cnt] = f->loc;
+ rule_locs[cnt] = f->fs.location;
cnt++;
}
if (!err)
@@ -158,7 +160,7 @@ static int idpf_add_flow_steer(struct net_device *netdev,
struct idpf_vport *vport;
u32 flow_type, q_index;
u16 num_rxq;
- int err;
+ int err = 0;
vport = idpf_netdev_to_vport(netdev);
vport_config = vport->adapter->vport_config[np->vport_idx];
@@ -184,6 +186,29 @@ static int idpf_add_flow_steer(struct net_device *netdev,
if (!rule)
return -ENOMEM;
+ fltr = kzalloc(sizeof(*fltr), GFP_KERNEL);
+ if (!fltr) {
+ err = -ENOMEM;
+ goto out_free_rule;
+ }
+
+ /* detect duplicate entry and reject before adding rules */
+ spin_lock_bh(&vport_config->flow_steer_list_lock);
+ list_for_each_entry(f, &user_config->flow_steer_list, list) {
+ if (f->fs.location == fsp->location) {
+ err = -EEXIST;
+ break;
+ }
+
+ if (f->fs.location > fsp->location)
+ break;
+ parent = f;
+ }
+ spin_unlock_bh(&vport_config->flow_steer_list_lock);
+
+ if (err)
+ goto out;
+
rule->vport_id = cpu_to_le32(vport->vport_id);
rule->count = cpu_to_le32(1);
info = &rule->rule_info[0];
@@ -222,28 +247,20 @@ static int idpf_add_flow_steer(struct net_device *netdev,
goto out;
}
- fltr = kzalloc(sizeof(*fltr), GFP_KERNEL);
- if (!fltr) {
- err = -ENOMEM;
- goto out;
- }
+ /* Save a copy of the user's flow spec so ethtool can later retrieve it */
+ fltr->fs = *fsp;
- fltr->loc = fsp->location;
- fltr->q_index = q_index;
spin_lock_bh(&vport_config->flow_steer_list_lock);
- list_for_each_entry(f, &user_config->flow_steer_list, list) {
- if (f->loc >= fltr->loc)
- break;
- parent = f;
- }
-
parent ? list_add(&fltr->list, &parent->list) :
list_add(&fltr->list, &user_config->flow_steer_list);
user_config->num_fsteer_fltrs++;
spin_unlock_bh(&vport_config->flow_steer_list_lock);
+ goto out_free_rule;
out:
+ kfree(fltr);
+out_free_rule:
kfree(rule);
return err;
}
@@ -297,14 +314,14 @@ static int idpf_del_flow_steer(struct net_device *netdev,
spin_lock_bh(&vport_config->flow_steer_list_lock);
list_for_each_entry_safe(f, iter,
&user_config->flow_steer_list, list) {
- if (f->loc == fsp->location) {
+ if (f->fs.location == fsp->location) {
list_del(&f->list);
kfree(f);
user_config->num_fsteer_fltrs--;
goto out_unlock;
}
}
- err = -EINVAL;
+ err = -ENOENT;
out_unlock:
spin_unlock_bh(&vport_config->flow_steer_list_lock);
--
2.25.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* RE: [PATCH] idpf: fix issue with ethtool -n command display
2025-09-25 15:24 [PATCH] idpf: fix issue with ethtool -n command display Sreedevi Joshi
@ 2025-09-25 15:27 ` Joshi, Sreedevi
0 siblings, 0 replies; 2+ messages in thread
From: Joshi, Sreedevi @ 2025-09-25 15:27 UTC (permalink / raw)
To: intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Carrillo, Erik G, Kitszel, Przemyslaw,
Loktionov, Aleksandr
> -----Original Message-----
> From: Joshi, Sreedevi <sreedevi.joshi@intel.com>
> Sent: Thursday, September 25, 2025 11:25 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Carrillo, Erik G <erik.g.carrillo@intel.com>; Joshi, Sreedevi <sreedevi.joshi@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Loktionov, Aleksandr <aleksandr.loktionov@intel.com>
> Subject: [PATCH] idpf: fix issue with ethtool -n command display
>
> From: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
>
> When ethtool -n is executed on an interface to display the flow steering
> rules, "rxclass: Unknown flow type" error is generated.
>
> The flow steering list maintained in the driver currently stores only the
> location and q_index but other fields of the ethtool_rx_flow_spec are not
> stored. This may be enough for the virtchnl command to delete the entry.
> However, when the ethtool -n command is used to query the flow steering
> rules, the ethtool_rx_flow_spec returned is not complete causing the
> error below.
>
> Resolve this by storing the flow spec (fsp) when rules are added and
> returning the complete flow spec when rules are queried.
>
> Also, change the return value from EINVAL to ENOENT when flow steering
> entry is not found during query by location or when deleting an entry.
>
> Add logic to detect and reject duplicate filter entries at the same
> location.
>
> Example:
> Before the fix:
> ethtool -n eth1
> 2 RX rings available
> Total 2 rules
>
> rxclass: Unknown flow type
> rxclass: Unknown flow type
>
> After the fix:
> ethtool -n eth1
> 2 RX rings available
> Total 2 rules
>
> Filter: 0
> Rule Type: TCP over IPv4
> Src IP addr: 10.0.0.1 mask: 0.0.0.0
> Dest IP addr: 0.0.0.0 mask: 255.255.255.255
> TOS: 0x0 mask: 0xff
> Src port: 0 mask: 0xffff
> Dest port: 0 mask: 0xffff
> Action: Direct to queue 0
>
> Filter: 1
> Rule Type: UDP over IPv4
> Src IP addr: 10.0.0.1 mask: 0.0.0.0
> Dest IP addr: 0.0.0.0 mask: 255.255.255.255
> TOS: 0x0 mask: 0xff
> Src port: 0 mask: 0xffff
> Dest port: 0 mask: 0xffff
> Action: Direct to queue 0
>
> Fixes: ada3e24b84a0 ("idpf: add flow steering support")
> Signed-off-by: Erik Gabriel Carrillo <erik.g.carrillo@intel.com>
> Co-developed-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> Signed-off-by: Sreedevi Joshi <sreedevi.joshi@intel.com>
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/idpf/idpf.h | 3 +-
> .../net/ethernet/intel/idpf/idpf_ethtool.c | 57 ++++++++++++-------
> 2 files changed, 38 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/idpf/idpf.h b/drivers/net/ethernet/intel/idpf/idpf.h
> index 4f4cf21e3c46..ec759bc5b3ce 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf.h
> +++ b/drivers/net/ethernet/intel/idpf/idpf.h
> @@ -269,8 +269,7 @@ struct idpf_port_stats {
>
> struct idpf_fsteer_fltr {
> struct list_head list;
> - u32 loc;
> - u32 q_index;
> + struct ethtool_rx_flow_spec fs;
> };
>
> /**
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> index 1352f18b60b0..6a39cc1feeb5 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c
> @@ -38,11 +38,13 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
> cmd->data = idpf_fsteer_max_rules(vport);
> break;
> case ETHTOOL_GRXCLSRULE:
> - err = -EINVAL;
> + err = -ENOENT;
> spin_lock_bh(&vport_config->flow_steer_list_lock);
> list_for_each_entry(f, &user_config->flow_steer_list, list)
> - if (f->loc == cmd->fs.location) {
> - cmd->fs.ring_cookie = f->q_index;
> + if (f->fs.location == cmd->fs.location) {
> + /* Avoid infoleak from padding: zero first, then assign fields */
> + memset(&cmd->fs, 0, sizeof(cmd->fs));
> + cmd->fs = f->fs;
> err = 0;
> break;
> }
> @@ -56,7 +58,7 @@ static int idpf_get_rxnfc(struct net_device *netdev, struct ethtool_rxnfc *cmd,
> err = -EMSGSIZE;
> break;
> }
> - rule_locs[cnt] = f->loc;
> + rule_locs[cnt] = f->fs.location;
> cnt++;
> }
> if (!err)
> @@ -158,7 +160,7 @@ static int idpf_add_flow_steer(struct net_device *netdev,
> struct idpf_vport *vport;
> u32 flow_type, q_index;
> u16 num_rxq;
> - int err;
> + int err = 0;
>
> vport = idpf_netdev_to_vport(netdev);
> vport_config = vport->adapter->vport_config[np->vport_idx];
> @@ -184,6 +186,29 @@ static int idpf_add_flow_steer(struct net_device *netdev,
> if (!rule)
> return -ENOMEM;
>
> + fltr = kzalloc(sizeof(*fltr), GFP_KERNEL);
> + if (!fltr) {
> + err = -ENOMEM;
> + goto out_free_rule;
> + }
> +
> + /* detect duplicate entry and reject before adding rules */
> + spin_lock_bh(&vport_config->flow_steer_list_lock);
> + list_for_each_entry(f, &user_config->flow_steer_list, list) {
> + if (f->fs.location == fsp->location) {
> + err = -EEXIST;
> + break;
> + }
> +
> + if (f->fs.location > fsp->location)
> + break;
> + parent = f;
> + }
> + spin_unlock_bh(&vport_config->flow_steer_list_lock);
> +
> + if (err)
> + goto out;
> +
> rule->vport_id = cpu_to_le32(vport->vport_id);
> rule->count = cpu_to_le32(1);
> info = &rule->rule_info[0];
> @@ -222,28 +247,20 @@ static int idpf_add_flow_steer(struct net_device *netdev,
> goto out;
> }
>
> - fltr = kzalloc(sizeof(*fltr), GFP_KERNEL);
> - if (!fltr) {
> - err = -ENOMEM;
> - goto out;
> - }
> + /* Save a copy of the user's flow spec so ethtool can later retrieve it */
> + fltr->fs = *fsp;
>
> - fltr->loc = fsp->location;
> - fltr->q_index = q_index;
> spin_lock_bh(&vport_config->flow_steer_list_lock);
> - list_for_each_entry(f, &user_config->flow_steer_list, list) {
> - if (f->loc >= fltr->loc)
> - break;
> - parent = f;
> - }
> -
> parent ? list_add(&fltr->list, &parent->list) :
> list_add(&fltr->list, &user_config->flow_steer_list);
>
> user_config->num_fsteer_fltrs++;
> spin_unlock_bh(&vport_config->flow_steer_list_lock);
> + goto out_free_rule;
>
> out:
> + kfree(fltr);
> +out_free_rule:
> kfree(rule);
> return err;
> }
> @@ -297,14 +314,14 @@ static int idpf_del_flow_steer(struct net_device *netdev,
> spin_lock_bh(&vport_config->flow_steer_list_lock);
> list_for_each_entry_safe(f, iter,
> &user_config->flow_steer_list, list) {
> - if (f->loc == fsp->location) {
> + if (f->fs.location == fsp->location) {
> list_del(&f->list);
> kfree(f);
> user_config->num_fsteer_fltrs--;
> goto out_unlock;
> }
> }
> - err = -EINVAL;
> + err = -ENOENT;
>
> out_unlock:
> spin_unlock_bh(&vport_config->flow_steer_list_lock);
> --
> 2.25.1
Please disregard this. Will resubmit the whole series.
Thanks
Sreedevi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-09-25 15:28 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-25 15:24 [PATCH] idpf: fix issue with ethtool -n command display Sreedevi Joshi
2025-09-25 15:27 ` Joshi, Sreedevi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox