Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints

[Roopa Prabhu]

On Tue, Aug 29, 2017 at 9:46 AM, Florian Fainelli  wrote:
> On 08/28/2017 09:22 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu 
>>
>> A few useful tracepoints to trace bridge forwarding
>> database updates.
>>
>> Signed-off-by: Roopa Prabhu 
>
> Reviewed-by: Florian Fainelli 
>
> Small nit below, but probably not a candidate for a v3
>
>> ---
>
>> +
>> + TP_printk("dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u nlh_flags 
>> %04x ndm_flags = %02x",
>
> Small nit, any particular reason why ndm_flags got a special treatment
> with an equal character and not the other?
>


good eyes, thats a typo. I did scan them once and removed the '=' and
missed a spot.
i will send v3.

Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints

[Florian Fainelli]

On 08/28/2017 09:22 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu 
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu 

Reviewed-by: Florian Fainelli 

Small nit below, but probably not a candidate for a v3

> ---

> +
> + TP_printk("dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u nlh_flags 
> %04x ndm_flags = %02x",

Small nit, any particular reason why ndm_flags got a special treatment
with an equal character and not the other?

> +   __get_str(dev), __entry->addr[0], __entry->addr[1],
> +   __entry->addr[2], __entry->addr[3], __entry->addr[4],
> +   __entry->addr[5], __entry->vid,
> +   __entry->nlh_flags, __entry->ndm_flags)
-- 
Florian

Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints

[Roopa Prabhu]

On Tue, Aug 29, 2017 at 6:36 AM, Andrew Lunn  wrote:
> On Mon, Aug 28, 2017 at 09:22:48PM -0700, Roopa Prabhu wrote:
>> From: Roopa Prabhu 
>>
>> A few useful tracepoints to trace bridge forwarding
>> database updates.
>
> Hi Roopa
>
> Once accepted, it would be nice to add mdb tracepoints as well. I
> expect the implementation would be very similar.
>

yes, sounds great. will do.

Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints

[Andrew Lunn]

On Mon, Aug 28, 2017 at 09:22:48PM -0700, Roopa Prabhu wrote:
> From: Roopa Prabhu 
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.

Hi Roopa

Once accepted, it would be nice to add mdb tracepoints as well. I
expect the implementation would be very similar.

   Andrew

Re: [PATCH net-next v2] bridge: fdb add and delete tracepoints

[Nikolay Aleksandrov]

On 29/08/17 07:22, Roopa Prabhu wrote:
> From: Roopa Prabhu 
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu 
> ---
> v2 - address comments from Florian
> 
>  include/trace/events/bridge.h |   98 
> +
>  net/bridge/br_fdb.c   |7 +++
>  net/core/net-traces.c |6 +++
>  3 files changed, 111 insertions(+)
>  create mode 100644 include/trace/events/bridge.h
> 

Very nice and useful, thanks!

Acked-by: Nikolay Aleksandrov 

[PATCH net-next v2] bridge: fdb add and delete tracepoints

[Roopa Prabhu]

From: Roopa Prabhu 

A few useful tracepoints to trace bridge forwarding
database updates.

Signed-off-by: Roopa Prabhu 
---
v2 - address comments from Florian

 include/trace/events/bridge.h |   98 +
 net/bridge/br_fdb.c   |7 +++
 net/core/net-traces.c |6 +++
 3 files changed, 111 insertions(+)
 create mode 100644 include/trace/events/bridge.h

diff --git a/include/trace/events/bridge.h b/include/trace/events/bridge.h
new file mode 100644
index 000..3a4ecc3
--- /dev/null
+++ b/include/trace/events/bridge.h
@@ -0,0 +1,98 @@
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM bridge
+
+#if !defined(_TRACE_BRIDGE_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_BRIDGE_H
+
+#include 
+#include 
+
+#include "../../../net/bridge/br_private.h"
+
+TRACE_EVENT(br_fdb_add,
+
+   TP_PROTO(struct ndmsg *ndm, struct net_device *dev,
+const unsigned char *addr, u16 vid, u16 nlh_flags),
+
+   TP_ARGS(ndm, dev, addr, vid, nlh_flags),
+
+   TP_STRUCT__entry(
+   __field(u8, ndm_flags)
+   __string(dev, dev->name)
+   __array(unsigned char, addr, ETH_ALEN)
+   __field(u16, vid)
+   __field(u16, nlh_flags)
+   ),
+
+   TP_fast_assign(
+   __assign_str(dev, dev->name);
+   memcpy(__entry->addr, addr, ETH_ALEN);
+   __entry->vid = vid;
+   __entry->nlh_flags = nlh_flags;
+   __entry->ndm_flags = ndm->ndm_flags;
+   ),
+
+   TP_printk("dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u nlh_flags 
%04x ndm_flags = %02x",
+ __get_str(dev), __entry->addr[0], __entry->addr[1],
+ __entry->addr[2], __entry->addr[3], __entry->addr[4],
+ __entry->addr[5], __entry->vid,
+ __entry->nlh_flags, __entry->ndm_flags)
+);
+
+TRACE_EVENT(br_fdb_external_learn_add,
+
+   TP_PROTO(struct net_bridge *br, struct net_bridge_port *p,
+const unsigned char *addr, u16 vid),
+
+   TP_ARGS(br, p, addr, vid),
+
+   TP_STRUCT__entry(
+   __string(br_dev, br->dev->name)
+   __string(dev, p->dev->name)
+   __array(unsigned char, addr, ETH_ALEN)
+   __field(u16, vid)
+   ),
+
+   TP_fast_assign(
+   __assign_str(br_dev, br ? br->dev->name : "null");
+   __assign_str(dev, p ? p->dev->name : "null");
+   memcpy(__entry->addr, addr, ETH_ALEN);
+   __entry->vid = vid;
+   ),
+
+   TP_printk("br_dev %s port %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u",
+ __get_str(br_dev), __get_str(dev), __entry->addr[0],
+ __entry->addr[1], __entry->addr[2], __entry->addr[3],
+ __entry->addr[4], __entry->addr[5], __entry->vid)
+);
+
+TRACE_EVENT(fdb_delete,
+
+   TP_PROTO(struct net_bridge *br, struct net_bridge_fdb_entry *f),
+
+   TP_ARGS(br, f),
+
+   TP_STRUCT__entry(
+   __string(br_dev, br->dev->name)
+   __string(dev, f->dst ? f->dst->dev->name : "null")
+   __array(unsigned char, addr, ETH_ALEN)
+   __field(u16, vid)
+   ),
+
+   TP_fast_assign(
+   __assign_str(br_dev, br ? br->dev->name : "null");
+   __assign_str(dev, f->dst ? f->dst->dev->name : "null");
+   memcpy(__entry->addr, f->addr.addr, ETH_ALEN);
+   __entry->vid = f->vlan_id;
+   ),
+
+   TP_printk("br_dev %s dev %s addr %02x:%02x:%02x:%02x:%02x:%02x vid %u",
+ __get_str(br_dev), __get_str(dev), __entry->addr[0],
+ __entry->addr[1], __entry->addr[2], __entry->addr[3],
+ __entry->addr[4], __entry->addr[5], __entry->vid)
+);
+
+#endif /* _TRACE_BRIDGE_H */
+
+/* This part must be outside protection */
+#include 
diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index a79b648..be5e1da 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "br_private.h"
 
 static struct kmem_cache *br_fdb_cache __read_mostly;
@@ -171,6 +172,8 @@ static void fdb_del_hw_addr(struct net_bridge *br, const 
unsigned char *addr)
 
 static void fdb_delete(struct net_bridge *br, struct net_bridge_fdb_entry *f)
 {
+   trace_fdb_delete(br, f);
+
if (f->is_static)
fdb_del_hw_addr(br, f->addr.addr);
 
@@ -870,6 +873,8 @@ int br_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
struct net_bridge *br = NULL;
int err = 0;
 
+   trace_br_fdb_add(ndm, dev, addr, vid, nlh_flags);
+
if (!(ndm->ndm_state & (NUD_PERMANENT|NUD_NOARP|NUD_REACHABLE))) {
pr_info("bridge: RTM_NEWNEIGH with invalid state %#x\n", 
ndm->ndm_state);
return -EINVAL;
@@ -1066,6 +1071,8 @@ int