All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ivan Vecera <ivecera@redhat.com>
To: Marcin Szycik <marcin.szycik@linux.intel.com>
Cc: netdev@vger.kernel.org, poros@redhat.com, mschmidt@redhat.com,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jeff Kirsher <jeffrey.t.kirsher@intel.com>,
	Akeem G Abodunrin <akeem.g.abodunrin@intel.com>,
	Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>,
	"moderated list:INTEL ETHERNET DRIVERS" 
	<intel-wired-lan@lists.osuosl.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH net] ice: Fix broken IFF_ALLMULTI handling
Date: Fri, 25 Mar 2022 14:04:24 +0100	[thread overview]
Message-ID: <20220325140424.696e8abc@ceranb> (raw)
In-Reply-To: <eb6538d9-4667-f1f5-492c-e1e113a6da35@linux.intel.com>

On Wed, 23 Mar 2022 21:05:20 +0100
Marcin Szycik <marcin.szycik@linux.intel.com> wrote:

> > @@ -3482,18 +3503,44 @@ ice_vlan_rx_kill_vid(struct net_device *netdev, __always_unused __be16 proto,
> >  	if (!vid)
> >  		return 0;
> >  
> > +	while (test_and_set_bit(ICE_CFG_BUSY, vsi->state))
> > +		usleep_range(1000, 2000);
> > +
> >  	/* Make sure ice_vsi_kill_vlan is successful before updating VLAN
> >  	 * information
> >  	 */
> >  	ret = ice_vsi_kill_vlan(vsi, vid);
> >  	if (ret)
> > -		return ret;
> > +		goto finish;
> >  
> > -	/* Disable pruning when VLAN 0 is the only VLAN rule */
> > -	if (vsi->num_vlan == 1 && ice_vsi_is_vlan_pruning_ena(vsi))
> > -		ret = ice_cfg_vlan_pruning(vsi, false);
> > +	/* Remove multicast promisc rule for the removed VLAN ID if
> > +	 * all-multicast is enabled.
> > +	 */
> > +	if (vsi->current_netdev_flags & IFF_ALLMULTI)
> > +		ice_fltr_clear_vsi_promisc(&vsi->back->hw, vsi->idx,
> > +					   ICE_MCAST_VLAN_PROMISC_BITS, vid);
> > +
> > +	if (vsi->num_vlan == 1) {
> > +		/* Disable pruning when VLAN 0 is the only VLAN rule */
> > +		if (ice_vsi_is_vlan_pruning_ena(vsi))
> > +			ice_cfg_vlan_pruning(vsi, false);  
> 
> Why was `ret = ...` removed here?

Because we are in ice_vlan_rx_kill_vid() that is used to remove VLAN and at this
the VLAN ID was removed from VLAN filter so we cannot return value other than 0.
Network stack would think that the VLAN ID is still present in VLAN filter.

Ivan


WARNING: multiple messages have this Message-ID (diff)
From: Ivan Vecera <ivecera@redhat.com>
To: intel-wired-lan@osuosl.org
Subject: [Intel-wired-lan] [PATCH net] ice: Fix broken IFF_ALLMULTI handling
Date: Fri, 25 Mar 2022 14:04:24 +0100	[thread overview]
Message-ID: <20220325140424.696e8abc@ceranb> (raw)
In-Reply-To: <eb6538d9-4667-f1f5-492c-e1e113a6da35@linux.intel.com>

On Wed, 23 Mar 2022 21:05:20 +0100
Marcin Szycik <marcin.szycik@linux.intel.com> wrote:

> > @@ -3482,18 +3503,44 @@ ice_vlan_rx_kill_vid(struct net_device *netdev, __always_unused __be16 proto,
> >  	if (!vid)
> >  		return 0;
> >  
> > +	while (test_and_set_bit(ICE_CFG_BUSY, vsi->state))
> > +		usleep_range(1000, 2000);
> > +
> >  	/* Make sure ice_vsi_kill_vlan is successful before updating VLAN
> >  	 * information
> >  	 */
> >  	ret = ice_vsi_kill_vlan(vsi, vid);
> >  	if (ret)
> > -		return ret;
> > +		goto finish;
> >  
> > -	/* Disable pruning when VLAN 0 is the only VLAN rule */
> > -	if (vsi->num_vlan == 1 && ice_vsi_is_vlan_pruning_ena(vsi))
> > -		ret = ice_cfg_vlan_pruning(vsi, false);
> > +	/* Remove multicast promisc rule for the removed VLAN ID if
> > +	 * all-multicast is enabled.
> > +	 */
> > +	if (vsi->current_netdev_flags & IFF_ALLMULTI)
> > +		ice_fltr_clear_vsi_promisc(&vsi->back->hw, vsi->idx,
> > +					   ICE_MCAST_VLAN_PROMISC_BITS, vid);
> > +
> > +	if (vsi->num_vlan == 1) {
> > +		/* Disable pruning when VLAN 0 is the only VLAN rule */
> > +		if (ice_vsi_is_vlan_pruning_ena(vsi))
> > +			ice_cfg_vlan_pruning(vsi, false);  
> 
> Why was `ret = ...` removed here?

Because we are in ice_vlan_rx_kill_vid() that is used to remove VLAN and at this
the VLAN ID was removed from VLAN filter so we cannot return value other than 0.
Network stack would think that the VLAN ID is still present in VLAN filter.

Ivan


  parent reply	other threads:[~2022-03-25 13:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 19:17 [PATCH net] ice: Fix broken IFF_ALLMULTI handling Ivan Vecera
2022-03-21 19:17 ` [Intel-wired-lan] " Ivan Vecera
2022-03-23 19:26 ` Keller, Jacob E
2022-03-23 20:05 ` Marcin Szycik
2022-03-23 20:05   ` [Intel-wired-lan] " Marcin Szycik
2022-03-25 12:53   ` Ivan Vecera
2022-03-25 12:53     ` [Intel-wired-lan] " Ivan Vecera
2022-03-25 13:04   ` Ivan Vecera [this message]
2022-03-25 13:04     ` Ivan Vecera

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220325140424.696e8abc@ceranb \
    --to=ivecera@redhat.com \
    --cc=akeem.g.abodunrin@intel.com \
    --cc=anirudh.venkataramanan@intel.com \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jeffrey.t.kirsher@intel.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marcin.szycik@linux.intel.com \
    --cc=mschmidt@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=poros@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.