[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <8DB4F573-128C-4A2D-A4D0-3909586AFF8C@bamaicloud.com>
Date: Mon, 23 Jun 2025 10:11:04 +0800
From: Tonghao Zhang <tonghao@...aicloud.com>
To: jv@...sburgh.net
Cc: netdev@...r.kernel.org,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Simon Horman <horms@...nel.org>,
Jonathan Corbet <corbet@....net>,
Andrew Lunn <andrew+netdev@...n.ch>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Nikolay Aleksandrov <razor@...ckwall.org>,
Zengbing Tu <tuzengbing@...iglobal.com>,
tonghao@...aicloud.com
Subject: Re: [net-next v6 4/4] net: bonding: add tracepoint for 802.3ad
> 2025年6月17日 18:37,Tonghao Zhang <tonghao@...aicloud.com> 写道:
>
>
>
>> 2025年6月17日 08:28,Jay Vosburgh <jv@...sburgh.net> 写道:
>>
>> Tonghao Zhang <tonghao@...aicloud.com> wrote:
>>
>>> Users can monitor NIC link status changes through netlink. However, LACP
>>> protocol failures may occur despite operational physical links. There is
>>> no way to detect LACP state changes. This patch adds tracepoint at
>>> LACP state transition.
>>
>> This patch really has nothing to do with the rest of the series
>> (it's unrelated to the broadcast_neighbor functionality), and should
>> really be sent separately.
> … monitoring the lacp state is part of “no-stacking” arch solution. So I sent it as series.
> if unnecessary, I will set it separately.
>
>> That said, I recall asking about work that was proposed some
> Sorry I may miss your commits about this patch.
>> time ago to create netlink events (visible to ip monitor, et al) when
>> the LACP state changes. That would be a cleaner method to watch the
>> LACP state machine (as it would integrate with all of the other event
> Why not consider a BPF+tracepoint solution? It provides more flexible LACP data collection with simpler implementation.
We developed a component. It collects kernel events via kprobe, ftrace, and tracepoint. Events include:
- Scheduling latency
- Direct memory reclaim
- Network packets drop
- LACP state events
BPF + tracepoint is our optimal approach. I think we should support this method.
>> infrastructure). Maybe I missed the response, but what became of that
>> work?
>>
>> -J
>>
>>> Cc: Jay Vosburgh <jv@...sburgh.net>
>>> Cc: "David S. Miller" <davem@...emloft.net>
>>> Cc: Eric Dumazet <edumazet@...gle.com>
>>> Cc: Jakub Kicinski <kuba@...nel.org>
>>> Cc: Paolo Abeni <pabeni@...hat.com>
>>> Cc: Simon Horman <horms@...nel.org>
>>> Cc: Jonathan Corbet <corbet@....net>
>>> Cc: Andrew Lunn <andrew+netdev@...n.ch>
>>> Cc: Steven Rostedt <rostedt@...dmis.org>
>>> Cc: Masami Hiramatsu <mhiramat@...nel.org>
>>> Cc: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
>>> Cc: Nikolay Aleksandrov <razor@...ckwall.org>
>>> Signed-off-by: Tonghao Zhang <tonghao@...aicloud.com>
>>> Signed-off-by: Zengbing Tu <tuzengbing@...iglobal.com>
>>> Reviewed-by: Nikolay Aleksandrov <razor@...ckwall.org>
>>> ---
>>> drivers/net/bonding/bond_3ad.c | 6 ++++++
>>> include/trace/events/bonding.h | 37 ++++++++++++++++++++++++++++++++++
>>> 2 files changed, 43 insertions(+)
>>> create mode 100644 include/trace/events/bonding.h
>>>
>>> diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
>>> index d1c2d416ac87..55703230ab29 100644
>>> --- a/drivers/net/bonding/bond_3ad.c
>>> +++ b/drivers/net/bonding/bond_3ad.c
>>> @@ -16,6 +16,9 @@
>>> #include <net/bond_3ad.h>
>>> #include <net/netlink.h>
>>>
>>> +#define CREATE_TRACE_POINTS
>>> +#include <trace/events/bonding.h>
>>> +
>>> /* General definitions */
>>> #define AD_SHORT_TIMEOUT 1
>>> #define AD_LONG_TIMEOUT 0
>>> @@ -1146,6 +1149,9 @@ static void ad_mux_machine(struct port *port, bool *update_slave_arr)
>>> port->actor_port_number,
>>> last_state,
>>> port->sm_mux_state);
>>> +
>>> + trace_3ad_mux_state(port->slave->dev, last_state, port->sm_mux_state);
>>> +
>>> switch (port->sm_mux_state) {
>>> case AD_MUX_DETACHED:
>>> port->actor_oper_port_state &= ~LACP_STATE_SYNCHRONIZATION;
>>> diff --git a/include/trace/events/bonding.h b/include/trace/events/bonding.h
>>> new file mode 100644
>>> index 000000000000..1ee4b07d912a
>>> --- /dev/null
>>> +++ b/include/trace/events/bonding.h
>>> @@ -0,0 +1,37 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 */
>>> +
>>> +#if !defined(_TRACE_BONDING_H) || defined(TRACE_HEADER_MULTI_READ)
>>> +#define _TRACE_BONDING_H
>>> +
>>> +#include <linux/netdevice.h>
>>> +#include <linux/tracepoint.h>
>>> +
>>> +#undef TRACE_SYSTEM
>>> +#define TRACE_SYSTEM bonding
>>> +
>>> +TRACE_EVENT(3ad_mux_state,
>>> + TP_PROTO(struct net_device *dev, u32 last_state, u32 curr_state),
>>> + TP_ARGS(dev, last_state, curr_state),
>>> +
>>> + TP_STRUCT__entry(
>>> + __field(int, ifindex)
>>> + __string(dev_name, dev->name)
>>> + __field(u32, last_state)
>>> + __field(u32, curr_state)
>>> + ),
>>> +
>>> + TP_fast_assign(
>>> + __entry->ifindex = dev->ifindex;
>>> + __assign_str(dev_name);
>>> + __entry->last_state = last_state;
>>> + __entry->curr_state = curr_state;
>>> + ),
>>> +
>>> + TP_printk("ifindex %d dev %s last_state 0x%x curr_state 0x%x",
>>> + __entry->ifindex, __get_str(dev_name),
>>> + __entry->last_state, __entry->curr_state)
>>> +);
>>> +
>>> +#endif /* _TRACE_BONDING_H */
>>> +
>>> +#include <trace/define_trace.h>
>>> --
>>> 2.34.1
>>>
>>
>> ---
>> -Jay Vosburgh, jv@...sburgh.net
Powered by blists - more mailing lists