Re: [PATCH net-next v1 2/9] net: dsa: tag_ar9331: detect IGMP and MLD packets

From: Oleksij Rempel <linux@rempel-privat.de>
To: Andrew Lunn <andrew@lunn.ch>, Oleksij Rempel <o.rempel@pengutronix.de>
Cc: Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Russell King <linux@armlinux.org.uk>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-mips@vger.kernel.org
Subject: Re: [PATCH net-next v1 2/9] net: dsa: tag_ar9331: detect IGMP and MLD packets
Date: Sat, 3 Apr 2021 19:14:56 +0200	[thread overview]
Message-ID: <f4856601-4219-09c7-2933-2161afd03abe@rempel-privat.de> (raw)
In-Reply-To: <YGiAjngOfDVWz/D7@lunn.ch>

Am 03.04.21 um 16:49 schrieb Andrew Lunn:
>> @@ -31,6 +96,13 @@ static struct sk_buff *ar9331_tag_xmit(struct sk_buff *skb,
>>  	__le16 *phdr;
>>  	u16 hdr;
>>
>> +	if (dp->stp_state == BR_STATE_BLOCKING) {
>> +		/* TODO: should we reflect it in the stats? */
>> +		netdev_warn_once(dev, "%s:%i dropping blocking packet\n",
>> +				 __func__, __LINE__);
>> +		return NULL;
>> +	}
>> +
>>  	phdr = skb_push(skb, AR9331_HDR_LEN);
>>
>>  	hdr = FIELD_PREP(AR9331_HDR_VERSION_MASK, AR9331_HDR_VERSION);
>
> Hi Oleksij
>
> This change does not seem to fit with what this patch is doing.

done

> I also think it is wrong. You still need BPDU to pass through a
> blocked port, otherwise spanning tree protocol will be unstable.

We need a better filter, otherwise, in case of software based STP, we are leaking packages on
blocked port. For example DHCP do trigger lots of spam in the kernel log.

I'll drop STP patch for now, it will be better to make a generic soft STP for all switches without
HW offloading. For example ksz9477 is doing SW based STP in similar way.

--
Regards,
Oleksij