* [PATCH] net/ice: fix disabling outer VLAN insertion
@ 2026-09-30 2:48 Anurag Mandal
2026-10-06 14:33 ` Burakov, Anatoly
0 siblings, 1 reply; 3+ messages in thread
From: Anurag Mandal @ 2026-09-30 2:48 UTC (permalink / raw)
To: dev; +Cc: bruce.richardson, anatoly.burakov, Anurag Mandal, stable
ice_vsi_dis_outer_insertion() treats a 0 return from ice_update_vsi()
as a failure, but a return value 0 means success, so the error and
success paths are swapped.
Removing a port VLAN in double VLAN mode logs an incorrect error,
returns invalid argument, and leaves a stale
ICE_AQ_VSI_OUTER_VLAN_PORT_BASED_INSERT bit in the cached
VSI context that later VSI updates replay into hardware.
Inverted the check to avoid this bug.
Fixes: de5da9d16430 ("net/ice: support double VLAN")
Cc: stable@dpdk.org
Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
---
drivers/net/intel/ice/ice_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
index 163304ba1e..2c5ffe07f3 100644
--- a/drivers/net/intel/ice/ice_ethdev.c
+++ b/drivers/net/intel/ice/ice_ethdev.c
@@ -6447,7 +6447,7 @@ static int ice_vsi_dis_outer_insertion(struct ice_vsi *vsi, struct ice_vsi_vlan_
ICE_AQ_VSI_OUTER_VLAN_TX_MODE_M);
status = ice_update_vsi(hw, vsi->idx, &ctxt, NULL);
- if (!status) {
+ if (status) {
PMD_DRV_LOG(ERR,
"update VSI for disabling outer VLAN insertion failed, err %d",
status);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net/ice: fix disabling outer VLAN insertion
2026-09-30 2:48 [PATCH] net/ice: fix disabling outer VLAN insertion Anurag Mandal
@ 2026-10-06 14:33 ` Burakov, Anatoly
2026-10-06 15:55 ` Bruce Richardson
0 siblings, 1 reply; 3+ messages in thread
From: Burakov, Anatoly @ 2026-10-06 14:33 UTC (permalink / raw)
To: Anurag Mandal, dev; +Cc: bruce.richardson, stable
On 9/30/2026 4:48 AM, Anurag Mandal wrote:
> ice_vsi_dis_outer_insertion() treats a 0 return from ice_update_vsi()
> as a failure, but a return value 0 means success, so the error and
> success paths are swapped.
> Removing a port VLAN in double VLAN mode logs an incorrect error,
> returns invalid argument, and leaves a stale
> ICE_AQ_VSI_OUTER_VLAN_PORT_BASED_INSERT bit in the cached
> VSI context that later VSI updates replay into hardware.
>
> Inverted the check to avoid this bug.
>
> Fixes: de5da9d16430 ("net/ice: support double VLAN")
> Cc: stable@dpdk.org
>
> Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> ---
> drivers/net/intel/ice/ice_ethdev.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> index 163304ba1e..2c5ffe07f3 100644
> --- a/drivers/net/intel/ice/ice_ethdev.c
> +++ b/drivers/net/intel/ice/ice_ethdev.c
> @@ -6447,7 +6447,7 @@ static int ice_vsi_dis_outer_insertion(struct ice_vsi *vsi, struct ice_vsi_vlan_
> ICE_AQ_VSI_OUTER_VLAN_TX_MODE_M);
>
> status = ice_update_vsi(hw, vsi->idx, &ctxt, NULL);
> - if (!status) {
> + if (status) {
> PMD_DRV_LOG(ERR,
> "update VSI for disabling outer VLAN insertion failed, err %d",
> status);
I'd have preferred we do not treat integer values as booleans. This
particular function returns the ice status code rather than int so
perhaps we should check for `if (status != ICE_SUCCESS)` instead.
However, this isn't a big deal and can probably be fixed on apply, so
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
--
Thanks,
Anatoly
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net/ice: fix disabling outer VLAN insertion
2026-10-06 14:33 ` Burakov, Anatoly
@ 2026-10-06 15:55 ` Bruce Richardson
0 siblings, 0 replies; 3+ messages in thread
From: Bruce Richardson @ 2026-10-06 15:55 UTC (permalink / raw)
To: Burakov, Anatoly; +Cc: Anurag Mandal, dev, stable
On Tue, Oct 06, 2026 at 04:33:50PM +0200, Burakov, Anatoly wrote:
> On 9/30/2026 4:48 AM, Anurag Mandal wrote:
> > ice_vsi_dis_outer_insertion() treats a 0 return from ice_update_vsi()
> > as a failure, but a return value 0 means success, so the error and
> > success paths are swapped.
> > Removing a port VLAN in double VLAN mode logs an incorrect error,
> > returns invalid argument, and leaves a stale
> > ICE_AQ_VSI_OUTER_VLAN_PORT_BASED_INSERT bit in the cached
> > VSI context that later VSI updates replay into hardware.
> >
> > Inverted the check to avoid this bug.
> >
> > Fixes: de5da9d16430 ("net/ice: support double VLAN")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Anurag Mandal <anurag.mandal@intel.com>
> > ---
> > drivers/net/intel/ice/ice_ethdev.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/intel/ice/ice_ethdev.c b/drivers/net/intel/ice/ice_ethdev.c
> > index 163304ba1e..2c5ffe07f3 100644
> > --- a/drivers/net/intel/ice/ice_ethdev.c
> > +++ b/drivers/net/intel/ice/ice_ethdev.c
> > @@ -6447,7 +6447,7 @@ static int ice_vsi_dis_outer_insertion(struct ice_vsi *vsi, struct ice_vsi_vlan_
> > ICE_AQ_VSI_OUTER_VLAN_TX_MODE_M);
> > status = ice_update_vsi(hw, vsi->idx, &ctxt, NULL);
> > - if (!status) {
> > + if (status) {
> > PMD_DRV_LOG(ERR,
> > "update VSI for disabling outer VLAN insertion failed, err %d",
> > status);
>
> I'd have preferred we do not treat integer values as booleans. This
> particular function returns the ice status code rather than int so perhaps
> we should check for `if (status != ICE_SUCCESS)` instead.
>
> However, this isn't a big deal and can probably be fixed on apply, so
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
Applied to dpdk-next-net-intel with suggested change applied. I too agree
that it's clearer to compare against ICE_SUCCESS
/Bruce
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-10-06 15:55 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30 2:48 [PATCH] net/ice: fix disabling outer VLAN insertion Anurag Mandal
2026-10-06 14:33 ` Burakov, Anatoly
2026-10-06 15:55 ` Bruce Richardson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox