netdev - Re: [PATCH net-next 06/16] net: bridge: Add a tracepoint for MDB overflows

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h6wc3um7.fsf@nvidia.com>
Date:   Fri, 27 Jan 2023 15:29:19 +0100
From:   Petr Machata <petrm@...dia.com>
To:     Steven Rostedt <rostedt@...dmis.org>
CC:     Petr Machata <petrm@...dia.com>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        "Paolo Abeni" <pabeni@...hat.com>, Roopa Prabhu <roopa@...dia.com>,
        "Nikolay Aleksandrov" <razor@...ckwall.org>,
        <netdev@...r.kernel.org>, <bridge@...ts.linux-foundation.org>,
        Ido Schimmel <idosch@...dia.com>,
        <linux-trace-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next 06/16] net: bridge: Add a tracepoint for MDB
 overflows


Steven Rostedt <rostedt@...dmis.org> writes:

>> diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
>> index 6b200059c2c5..00d5e2dcb3ad 100644
>> --- a/include/trace/events/bridge.h
>> +++ b/include/trace/events/bridge.h
>> @@ -122,6 +122,73 @@ TRACE_EVENT(br_fdb_update,
>>  		  __entry->flags)
>>  );
>>  
>> +TRACE_EVENT(br_mdb_full,
>> +
>> +	TP_PROTO(const struct net_device *dev,
>> +		 const struct br_ip *group),
>> +
>> +	TP_ARGS(dev, group),
>> +
>> +	TP_STRUCT__entry(
>> +		__string(dev, dev->name)
>> +		__field(int, af)
>> +		__field(u16, vid)
>> +		__array(__u8, src4, 4)
>> +		__array(__u8, src6, 16)
>> +		__array(__u8, grp4, 4)
>> +		__array(__u8, grp6, 16)
>> +		__array(__u8, grpmac, ETH_ALEN) /* For af == 0. */
>
> Instead of wasting ring buffer space, why not just have:
>
> 		__array(__u8, src, 16)
> 		__array(__u8, grp, 16)
>
>> +	),
>> +
>> +	TP_fast_assign(
>> +		__assign_str(dev, dev->name);
>> +		__entry->vid = group->vid;
>> +
>> +		if (!group->proto) {
>> +			__entry->af = 0;
>> +
>> +			memset(__entry->src4, 0, sizeof(__entry->src4));
>> +			memset(__entry->src6, 0, sizeof(__entry->src6));
>> +			memset(__entry->grp4, 0, sizeof(__entry->grp4));
>> +			memset(__entry->grp6, 0, sizeof(__entry->grp6));
>> +			memcpy(__entry->grpmac, group->dst.mac_addr, ETH_ALEN);
>> +		} else if (group->proto == htons(ETH_P_IP)) {
>> +			__be32 *p32;
>> +
>> +			__entry->af = AF_INET;
>> +
>> +			p32 = (__be32 *) __entry->src4;
>> +			*p32 = group->src.ip4;
>> +
>> +			p32 = (__be32 *) __entry->grp4;
>> +			*p32 = group->dst.ip4;
>
> 			struct in6_addr *in6;
>
> 			in6 = (struct in6_addr *)__entry->src;
> 			ipv6_addr_set_v4mapped(group->src.ip4, in6);
>
> 			in6 = (struct in6_addr *)__entry->grp;
> 			ipv6_addr_set_v4mapped(group->grp.ip4, in6);
>
>> +
>> +			memset(__entry->src6, 0, sizeof(__entry->src6));
>> +			memset(__entry->grp6, 0, sizeof(__entry->grp6));
>> +			memset(__entry->grpmac, 0, ETH_ALEN);
>> +#if IS_ENABLED(CONFIG_IPV6)
>> +		} else {
>> +			struct in6_addr *in6;
>> +
>> +			__entry->af = AF_INET6;
>> +
>> +			in6 = (struct in6_addr *)__entry->src6;
>> +			*in6 = group->src.ip6;
>> +
>> +			in6 = (struct in6_addr *)__entry->grp6;
>> +			*in6 = group->dst.ip6;
>> +
>> +			memset(__entry->src4, 0, sizeof(__entry->src4));
>> +			memset(__entry->grp4, 0, sizeof(__entry->grp4));
>> +			memset(__entry->grpmac, 0, ETH_ALEN);
>> +#endif
>> +		}
>> +	),
>> +
>> +	TP_printk("dev %s af %u src %pI4/%pI6c grp %pI4/%pI6c/%pM vid %u",
>> +		  __get_str(dev), __entry->af, __entry->src4, __entry->src6,
>> +		  __entry->grp4, __entry->grp6, __entry->grpmac, __entry->vid)
>
> And just have: 
>
> 	TP_printk("dev %s af %u src %pI6c grp %pI6c/%pM vid %u",
> 		  __get_str(dev), __entry->af, __entry->src, __entry->grp,
> 		  __entry->grpmac, __entry->vid)
>
> As the %pI6c should detect that it's a ipv4 address and show that.

So the reason I split the fields was that %pI4, %pI6c, %pM do not seem
to work with buffers of wrong size.

But I can consolidate 4/6 by changing the address to IPv6 like you
propose. I'll do this for v2. Thanks!