net/iavf: remove error logs for vlan offloading

Message ID 20240206095607.339410-1-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Headers
Series net/iavf: remove error logs for vlan offloading |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/Intel-compilation success Compilation OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/intel-Functional success Functional PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-compile-amd64-testing success Testing PASS
ci/iol-sample-apps-testing success Testing PASS
ci/iol-unit-arm64-testing success Testing PASS
ci/iol-unit-amd64-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-compile-arm64-testing success Testing PASS

Commit Message

David Marchand Feb. 6, 2024, 9:56 a.m. UTC
  This was reported by RH QE.

When a vlan is enforced on a VF via an administrative configuration on
the PF side, the net/iavf driver logs two error messages.
Those error messages have no consequence on the rest of the port
initialisation and packet processing works fine.

[root@toto ~] # ip l set enp94s0 vf 0 vlan 2
[root@toto ~] # dpdk-testpmd -a 0000:5e:02.0 -- -i
...
Configuring Port 0 (socket 0)
iavf_dev_init_vlan(): Failed to update vlan offload
iavf_dev_configure(): configure VLAN failed: -95
iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
	rx_pkt_burst as legacy for all queues

The first change is to remove the error log in iavf_dev_init_vlan().
This log is unneeded since all error path are covered by a dedicated log
message already.

Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
must not trigger an ERROR level log message. This is simply confusing,
as the application may not have requested such vlan offloading.
The reason why the driver requests all offloading is unclear so keep it
as is. Instead, rephrase the log message and lower its level to INFO.

Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
Cc: stable@dpdk.org

Signed-off-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/iavf/iavf_ethdev.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)
  

Comments

Bruce Richardson Feb. 6, 2024, 10:03 a.m. UTC | #1
On Tue, Feb 06, 2024 at 10:56:07AM +0100, David Marchand wrote:
> This was reported by RH QE.
> 
> When a vlan is enforced on a VF via an administrative configuration on
> the PF side, the net/iavf driver logs two error messages.
> Those error messages have no consequence on the rest of the port
> initialisation and packet processing works fine.
> 
> [root@toto ~] # ip l set enp94s0 vf 0 vlan 2
> [root@toto ~] # dpdk-testpmd -a 0000:5e:02.0 -- -i
> ...
> Configuring Port 0 (socket 0)
> iavf_dev_init_vlan(): Failed to update vlan offload
> iavf_dev_configure(): configure VLAN failed: -95
> iavf_set_rx_function(): request RXDID[1] in Queue[0] is legacy, set
> 	rx_pkt_burst as legacy for all queues
> 
> The first change is to remove the error log in iavf_dev_init_vlan().
> This log is unneeded since all error path are covered by a dedicated log
> message already.
> 
> Then, in iavf_dev_init_vlan(), requesting all possible VLAN offloading
> must not trigger an ERROR level log message. This is simply confusing,
> as the application may not have requested such vlan offloading.
> The reason why the driver requests all offloading is unclear so keep it
> as is. Instead, rephrase the log message and lower its level to INFO.
> 
> Fixes: 1c301e8c3cff ("net/iavf: support new VLAN capabilities")
> Cc: stable@dpdk.org
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

One small suggestion inline below.
> ---
>  drivers/net/iavf/iavf_ethdev.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index 1fb876e827..fc92cdf146 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -631,7 +631,7 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
>  					RTE_ETH_VLAN_FILTER_MASK |
>  					RTE_ETH_VLAN_EXTEND_MASK);
>  	if (err) {
> -		PMD_DRV_LOG(ERR, "Failed to update vlan offload");
> +		PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN offloading");

Minor nit, the phrase "no support" seems ambiguous on first reading, since
it's not completely clear that the no support refers to vlan offloading.

How about:
  "VLAN offloading is not supported, or offloading was refused by the PF"

>  		return err;
>  	}
>  
> @@ -707,9 +707,7 @@ iavf_dev_configure(struct rte_eth_dev *dev)
>  		vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
>  	}
>  
> -	ret = iavf_dev_init_vlan(dev);
> -	if (ret)
> -		PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
> +	iavf_dev_init_vlan(dev);
>  
>  	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
>  		if (iavf_init_rss(ad) != 0) {
> -- 
> 2.43.0
>
  
David Marchand Feb. 6, 2024, 10:20 a.m. UTC | #2
On Tue, Feb 6, 2024 at 11:04 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
> > @@ -631,7 +631,7 @@ iavf_dev_init_vlan(struct rte_eth_dev *dev)
> >                                       RTE_ETH_VLAN_FILTER_MASK |
> >                                       RTE_ETH_VLAN_EXTEND_MASK);
> >       if (err) {
> > -             PMD_DRV_LOG(ERR, "Failed to update vlan offload");
> > +             PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN offloading");
>
> Minor nit, the phrase "no support" seems ambiguous on first reading, since
> it's not completely clear that the no support refers to vlan offloading.
>
> How about:
>   "VLAN offloading is not supported, or offloading was refused by the PF"

It reads better, I'll send a v2.
Thanks Bruce.
  

Patch

diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
index 1fb876e827..fc92cdf146 100644
--- a/drivers/net/iavf/iavf_ethdev.c
+++ b/drivers/net/iavf/iavf_ethdev.c
@@ -631,7 +631,7 @@  iavf_dev_init_vlan(struct rte_eth_dev *dev)
 					RTE_ETH_VLAN_FILTER_MASK |
 					RTE_ETH_VLAN_EXTEND_MASK);
 	if (err) {
-		PMD_DRV_LOG(ERR, "Failed to update vlan offload");
+		PMD_DRV_LOG(INFO, "There is no support or the PF refused VLAN offloading");
 		return err;
 	}
 
@@ -707,9 +707,7 @@  iavf_dev_configure(struct rte_eth_dev *dev)
 		vf->max_rss_qregion = IAVF_MAX_NUM_QUEUES_DFLT;
 	}
 
-	ret = iavf_dev_init_vlan(dev);
-	if (ret)
-		PMD_DRV_LOG(ERR, "configure VLAN failed: %d", ret);
+	iavf_dev_init_vlan(dev);
 
 	if (vf->vf_res->vf_cap_flags & VIRTCHNL_VF_OFFLOAD_RSS_PF) {
 		if (iavf_init_rss(ad) != 0) {