netdev - Re: [PATCH net-next v2 2/2] net: bridge: add skb drop reasons to the most common drop points

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <0b0dd0fa-cf7b-4654-ab25-3e6d80615bf1@blackwall.org>
Date: Thu, 19 Dec 2024 00:50:27 +0200
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Ido Schimmel <idosch@...sch.org>, Radu Rendec <rrendec@...hat.com>
Cc: Roopa Prabhu <roopa@...dia.com>, bridge@...ts.linux.dev,
 netdev@...r.kernel.org, Simon Horman <horms@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v2 2/2] net: bridge: add skb drop reasons to the
 most common drop points

On 12/18/24 19:19, Ido Schimmel wrote:
> On Tue, Dec 17, 2024 at 06:07:11PM -0500, Radu Rendec wrote:
>> @@ -520,6 +522,16 @@ enum skb_drop_reason {
>>  	 * enabled.
>>  	 */
>>  	SKB_DROP_REASON_ARP_PVLAN_DISABLE,
>> +	/**
>> +	 * @SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL: the destination MAC address
>> +	 * is an IEEE MAC Control address.
>> +	 */
>> +	SKB_DROP_REASON_MAC_IEEE_MAC_CONTROL,
>> +	/**
>> +	 * @SKB_DROP_REASON_BRIDGE_INGRESS_PORT_NFWD: the STP state of the
> 
> s/SKB_DROP_REASON_BRIDGE_INGRESS_PORT_NFWD/SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE/
> 
>> +	 * ingress bridge port does not allow frames to be forwarded.
>> +	 */
>> +	SKB_DROP_REASON_BRIDGE_INGRESS_STP_STATE,
>>  	/**
>>  	 * @SKB_DROP_REASON_MAX: the maximum of core drop reasons, which
>>  	 * shouldn't be used as a real 'reason' - only for tracing code gen
>> diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
>> index e19b583ff2c6d..3e9b462809b0e 100644
>> --- a/net/bridge/br_forward.c
>> +++ b/net/bridge/br_forward.c
>> @@ -201,6 +201,7 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
>>  	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig,
>>  	      u16 vid)
>>  {
>> +	enum skb_drop_reason reason = SKB_DROP_REASON_NO_TX_TARGET;
>>  	struct net_bridge_port *prev = NULL;
>>  	struct net_bridge_port *p;
>>  
>> @@ -234,8 +235,11 @@ void br_flood(struct net_bridge *br, struct sk_buff *skb,
>>  			continue;
>>  
>>  		prev = maybe_deliver(prev, p, skb, local_orig);
>> -		if (IS_ERR(prev))
>> +		if (IS_ERR(prev)) {
>> +			WARN_ON_ONCE(PTR_ERR(prev) != -ENOMEM);
> 
> I don't think we want to see a stack trace just because someone forgot
> to adjust the drop reason to the error code. Maybe just set it to
> 'NOMEM' if error code is '-ENOMEM', otherwise to 'NOT_SPECIFIED'.
> 

+1