Hi Ying,
We discussed this, and even had a first version where we did this, performing
the whole attach/detach tasks in the command execution itself. This resulted in
way to much new code to my taste.
My greatest concern now is that some users will enable this interface because
they think it is necessary (we have already seen cases of that) and then start
complaining about performance.
So, as a compromise, I suggested we could add a "confirmation" printout in the
tipc tool when the loopback interface is enabled, to make the user understand
that this is for tracing only.
Do you think this would be acceptable?
///jon
> -----Original Message-----
> From: Ying Xue <ying....@windriver.com>
> Sent: 21-Jun-19 08:16
> To: John Rutherford <john.rutherf...@dektech.com.au>; tipc-
> discuss...@lists.sourceforge.net
> Subject: Re: [tipc-discussion] [net-next v2] tipc: add loopback device
> tracking
>
> Good work!
>
> Just one suggestion: it's better to add one separate kernel config to control
> whether the new feature is enabled or not, and its default value should be set
> to "Disabled" because the feature is related to debug.
>
> On 6/19/19 8:11 AM, John Rutherford wrote:
> > Since node internal messages are passed directly to socket it is not
> > possible to observe this message exchange via tcpdump or wireshark.
> >
> > We now remedy this by making it possible to clone such messages and
> > send the clones to the loopback interface. The clones are dropped at
> > reception and have no functional role except making the traffic visible.
> >
> > The feature is turned on/off by enabling/disabling the loopback "bearer"
> > "eth:lo".
> >
> > Signed-off-by: John Rutherford <john.rutherf...@dektech.com.au>
> > ---
> > net/tipc/bcast.c | 4 +++-
> > net/tipc/bearer.c | 67
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > net/tipc/bearer.h | 3 +++
> > net/tipc/core.c | 5 ++++-
> > net/tipc/core.h | 12 ++++++++++
> > net/tipc/node.c | 1 +
> > net/tipc/topsrv.c | 2 ++
> > 7 files changed, 92 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/tipc/bcast.c b/net/tipc/bcast.c index
> > 6c997d4..235331d 100644
> > --- a/net/tipc/bcast.c
> > +++ b/net/tipc/bcast.c
> > @@ -406,8 +406,10 @@ int tipc_mcast_xmit(struct net *net, struct
> sk_buff_head *pkts,
> > rc = tipc_bcast_xmit(net, pkts, cong_link_cnt);
> > }
> >
> > - if (dests->local)
> > + if (dests->local) {
> > + tipc_loopback_trace(net, &localq);
> > tipc_sk_mcast_rcv(net, &localq, &inputq);
> > + }
> > exit:
> > /* This queue should normally be empty by now */
> > __skb_queue_purge(pkts);
> > diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c index
> > 2bed658..27b4fd7 100644
> > --- a/net/tipc/bearer.c
> > +++ b/net/tipc/bearer.c
> > @@ -836,6 +836,12 @@ int __tipc_nl_bearer_disable(struct sk_buff *skb,
> > struct genl_info *info)
> >
> > name = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
> >
> > + if (!strcmp(name, "eth:lo")) {
> > + tipc_net(net)->loopback_trace = false;
> > + pr_info("Disabled packet tracing on loopback interface\n");
> > + return 0;
> > + }
> > +
> > bearer = tipc_bearer_find(net, name);
> > if (!bearer)
> > return -EINVAL;
> > @@ -881,6 +887,12 @@ int __tipc_nl_bearer_enable(struct sk_buff *skb,
> > struct genl_info *info)
> >
> > bearer = nla_data(attrs[TIPC_NLA_BEARER_NAME]);
> >
> > + if (!strcmp(bearer, "eth:lo")) {
> > + tipc_net(net)->loopback_trace = true;
> > + pr_info("Enabled packet tracing on loopback interface\n");
> > + return 0;
> > + }
> > +
> > if (attrs[TIPC_NLA_BEARER_DOMAIN])
> > domain = nla_get_u32(attrs[TIPC_NLA_BEARER_DOMAIN]);
> >
> > @@ -1021,6 +1033,61 @@ int tipc_nl_bearer_set(struct sk_buff *skb,
> struct genl_info *info)
> > return err;
> > }
> >
> > +void tipc_clone_to_loopback(struct net *net, struct sk_buff_head
> > +*xmitq) {
> > + struct net_device *dev = net->loopback_dev;
> > + struct sk_buff *skb, *_skb;
> > + int exp;
> > +
> > + skb_queue_walk(xmitq, _skb) {
> > + skb = pskb_copy(_skb, GFP_ATOMIC);
> > + if (!skb)
> > + continue;
> > + exp = SKB_DATA_ALIGN(dev->hard_header_len -
> skb_headroom(skb));
> > + if (exp > 0 && pskb_expand_head(skb, exp, 0, GFP_ATOMIC)) {
> > + kfree_skb(skb);
> > + continue;
> > + }
> > + skb_reset_network_header(skb);
> > + skb->dev = dev;
> > + skb->protocol = htons(ETH_P_TIPC);
> > + dev_hard_header(skb, dev, ETH_P_TIPC, dev->dev_addr,
> > + dev->dev_addr, skb->len);
> > + dev_queue_xmit(skb);
> > + }
> > +}
> > +
> > +static int tipc_loopback_rcv_pkt(struct sk_buff *skb, struct net_device
> *dev,
> > + struct packet_type *pt, struct net_device *od)
> > {
> > + consume_skb(skb);
> > + return NET_RX_SUCCESS;
> > +}
> > +
> > +int tipc_attach_loopback(struct net *net) {
> > + struct net_device *dev = net->loopback_dev;
> > + struct tipc_net *tn = tipc_net(net);
> > +
> > + if (!dev)
> > + return -ENODEV;
> > + dev_hold(dev);
> > + tn->loopback_pt.dev = dev;
> > + tn->loopback_pt.type = htons(ETH_P_TIPC);
> > + tn->loopback_pt.func = tipc_loopback_rcv_pkt;
> > + tn->loopback_trace = false;
> > + dev_add_pack(&tn->loopback_pt);
> > + return 0;
> > +}
> > +
> > +void tipc_detach_loopback(struct net *net) {
> > + struct tipc_net *tn = tipc_net(net);
> > +
> > + dev_remove_pack(&tn->loopback_pt);
> > + dev_put(net->loopback_dev);
> > +}
> > +
> > static int __tipc_nl_add_media(struct tipc_nl_msg *msg,
> > struct tipc_media *media, int nlflags) { diff
> > --git
> > a/net/tipc/bearer.h b/net/tipc/bearer.h index 7f4c569..ef7fad9 100644
> > --- a/net/tipc/bearer.h
> > +++ b/net/tipc/bearer.h
> > @@ -232,6 +232,9 @@ void tipc_bearer_xmit(struct net *net, u32
> bearer_id,
> > struct tipc_media_addr *dst); void
> > tipc_bearer_bc_xmit(struct net *net, u32 bearer_id,
> > struct sk_buff_head *xmitq);
> > +void tipc_clone_to_loopback(struct net *net, struct sk_buff_head
> > +*xmitq); int tipc_attach_loopback(struct net *net); void
> > +tipc_detach_loopback(struct net *net);
> >
> > /* check if device MTU is too low for tipc headers */ static inline
> > bool tipc_mtu_bad(struct net_device *dev, unsigned int reserve) diff
> > --git a/net/tipc/core.c b/net/tipc/core.c index ed536c0..1867687
> > 100644
> > --- a/net/tipc/core.c
> > +++ b/net/tipc/core.c
> > @@ -81,7 +81,9 @@ static int __net_init tipc_init_net(struct net *net)
> > err = tipc_bcast_init(net);
> > if (err)
> > goto out_bclink;
> > -
> > + err = tipc_attach_loopback(net);
> > + if (err)
> > + goto out_bclink;
> > return 0;
> >
> > out_bclink:
> > @@ -94,6 +96,7 @@ static int __net_init tipc_init_net(struct net *net)
> >
> > static void __net_exit tipc_exit_net(struct net *net) {
> > + tipc_detach_loopback(net);
> > tipc_net_stop(net);
> > tipc_bcast_stop(net);
> > tipc_nametbl_stop(net);
> > diff --git a/net/tipc/core.h b/net/tipc/core.h index 7a68e1b..c1c2906
> > 100644
> > --- a/net/tipc/core.h
> > +++ b/net/tipc/core.h
> > @@ -67,6 +67,7 @@ struct tipc_link;
> > struct tipc_name_table;
> > struct tipc_topsrv;
> > struct tipc_monitor;
> > +void tipc_clone_to_loopback(struct net *net, struct sk_buff_head
> > +*pkts);
> >
> > #define TIPC_MOD_VER "2.0.0"
> >
> > @@ -125,6 +126,10 @@ struct tipc_net {
> >
> > /* Cluster capabilities */
> > u16 capabilities;
> > +
> > + /* Tracing of node internal messages */
> > + struct packet_type loopback_pt;
> > + bool loopback_trace;
> > };
> >
> > static inline struct tipc_net *tipc_net(struct net *net) @@ -152,6
> > +157,13 @@ static inline struct tipc_topsrv *tipc_topsrv(struct net *net)
> > return tipc_net(net)->topsrv;
> > }
> >
> > +static inline void tipc_loopback_trace(struct net *net,
> > + struct sk_buff_head *pkts) {
> > + if (unlikely(tipc_net(net)->loopback_trace))
> > + tipc_clone_to_loopback(net, pkts);
> > +}
> > +
> > static inline unsigned int tipc_hashfn(u32 addr) {
> > return addr & (NODE_HTABLE_SIZE - 1); diff --git a/net/tipc/node.c
> > b/net/tipc/node.c index 9e106d3..7e58831 100644
> > --- a/net/tipc/node.c
> > +++ b/net/tipc/node.c
> > @@ -1439,6 +1439,7 @@ int tipc_node_xmit(struct net *net, struct
> sk_buff_head *list,
> > int rc;
> >
> > if (in_own_node(net, dnode)) {
> > + tipc_loopback_trace(net, list);
> > tipc_sk_rcv(net, list);
> > return 0;
> > }
> > diff --git a/net/tipc/topsrv.c b/net/tipc/topsrv.c index
> > f345662..e3a6ba1 100644
> > --- a/net/tipc/topsrv.c
> > +++ b/net/tipc/topsrv.c
> > @@ -40,6 +40,7 @@
> > #include "socket.h"
> > #include "addr.h"
> > #include "msg.h"
> > +#include "bearer.h"
> > #include <net/sock.h>
> > #include <linux/module.h>
> >
> > @@ -608,6 +609,7 @@ static void tipc_topsrv_kern_evt(struct net *net,
> struct tipc_event *evt)
> > memcpy(msg_data(buf_msg(skb)), evt, sizeof(*evt));
> > skb_queue_head_init(&evtq);
> > __skb_queue_tail(&evtq, skb);
> > + tipc_loopback_trace(net, &evtq);
> > tipc_sk_rcv(net, &evtq);
> > }
> >
> >
>
>
> _______________________________________________
> tipc-discussion mailing list
> tipc-discussion@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tipc-discussion
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
