All of lore.kernel.org
 help / color / mirror / Atom feed
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 13:53:37 +0100	[thread overview]
Message-ID: <20220325135337.2091ba08@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:

> > @@ -352,29 +359,15 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi)
> >  	/* check for changes in promiscuous modes */
> >  	if (changed_flags & IFF_ALLMULTI) {
> >  		if (vsi->current_netdev_flags & IFF_ALLMULTI) {
> > -			if (vsi->num_vlan > 1)
> > -				promisc_m = ICE_MCAST_VLAN_PROMISC_BITS;
> > -			else
> > -				promisc_m = ICE_MCAST_PROMISC_BITS;  
> 
> Because `ice_{set,clear}_promisc()` are now always called with the same second argument (ICE_MCAST_PROMISC_BITS), wouldn't it be better to remove the arg and instead call `ice_fltr_{clear,set}_{vlan,vsi}_vsi_promisc()` with either ICE_MCAST_VLAN_PROMISC_BITS or ICE_MCAST_PROMISC_BITS inside the function?

Because ice_{set,clear}_promisc() then could be used only for set mcast prosmisc mode so I modified them only to automatically insert ICE_PROMISC_VLAN_RX & ICE_PROMISC_VLAN_TX based on vsi->num_vlan value.

Anyway I will fix some objections from Jacob and send v2.

Thanks,
Ivan


WARNING: multiple messages have this Message-ID (diff)
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 13:53:37 +0100	[thread overview]
Message-ID: <20220325135337.2091ba08@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:

> > @@ -352,29 +359,15 @@ static int ice_vsi_sync_fltr(struct ice_vsi *vsi)
> >  	/* check for changes in promiscuous modes */
> >  	if (changed_flags & IFF_ALLMULTI) {
> >  		if (vsi->current_netdev_flags & IFF_ALLMULTI) {
> > -			if (vsi->num_vlan > 1)
> > -				promisc_m = ICE_MCAST_VLAN_PROMISC_BITS;
> > -			else
> > -				promisc_m = ICE_MCAST_PROMISC_BITS;  
> 
> Because `ice_{set,clear}_promisc()` are now always called with the same second argument (ICE_MCAST_PROMISC_BITS), wouldn't it be better to remove the arg and instead call `ice_fltr_{clear,set}_{vlan,vsi}_vsi_promisc()` with either ICE_MCAST_VLAN_PROMISC_BITS or ICE_MCAST_PROMISC_BITS inside the function?

Because ice_{set,clear}_promisc() then could be used only for set mcast prosmisc mode so I modified them only to automatically insert ICE_PROMISC_VLAN_RX & ICE_PROMISC_VLAN_TX based on vsi->num_vlan value.

Anyway I will fix some objections from Jacob and send v2.

Thanks,
Ivan


  reply	other threads:[~2022-03-25 12:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-21 19:17 [Intel-wired-lan] [PATCH net] ice: Fix broken IFF_ALLMULTI handling Ivan Vecera
2022-03-21 19:17 ` Ivan Vecera
2022-03-23 19:26 ` [Intel-wired-lan] " Keller, Jacob E
2022-03-23 20:05 ` Marcin Szycik
2022-03-23 20:05   ` Marcin Szycik
2022-03-25 12:53   ` Ivan Vecera [this message]
2022-03-25 12:53     ` Ivan Vecera
2022-03-25 13:04   ` [Intel-wired-lan] " Ivan Vecera
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=20220325135337.2091ba08@ceranb \
    --to=ivecera@redhat.com \
    --cc=intel-wired-lan@osuosl.org \
    /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.