| Message ID | 20240821133714.61417-4-przemyslaw.kitszel@intel.com (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ice: add support for devlink health events | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
On Wed, Aug 21, 2024 at 03:37:11PM +0200, Przemek Kitszel wrote: > From: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > > Add devlink_fmsg_dump_skb() function that adds some diagnostic > information about skb (like length, pkt type, MAC, etc) to devlink > fmsg mechanism using bunch of devlink_fmsg_put() function calls. > > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> ... > diff --git a/net/devlink/health.c b/net/devlink/health.c > index acb8c0e174bb..b98ca650284c 100644 > --- a/net/devlink/health.c > +++ b/net/devlink/health.c > @@ -1241,3 +1241,70 @@ int devlink_nl_health_reporter_test_doit(struct sk_buff *skb, > > return reporter->ops->test(reporter, info->extack); > } > + > +/** > + * devlink_fmsg_dump_skb - Dump sk_buffer structure > + * @fmsg: devlink formatted message pointer > + * @skb: pointer to skb > + * > + * Dump diagnostic information about sk_buff structure, like headroom, length, > + * tailroom, MAC, etc. > + */ > +void devlink_fmsg_dump_skb(struct devlink_fmsg *fmsg, const struct sk_buff *skb) > +{ > + struct skb_shared_info *sh = skb_shinfo(skb); > + struct sock *sk = skb->sk; > + bool has_mac, has_trans; > + > + has_mac = skb_mac_header_was_set(skb); > + has_trans = skb_transport_header_was_set(skb); > + > + devlink_fmsg_pair_nest_start(fmsg, "skb"); > + devlink_fmsg_obj_nest_start(fmsg); > + devlink_fmsg_put(fmsg, "actual len", skb->len); > + devlink_fmsg_put(fmsg, "head len", skb_headlen(skb)); > + devlink_fmsg_put(fmsg, "data len", skb->data_len); > + devlink_fmsg_put(fmsg, "tail len", skb_tailroom(skb)); > + devlink_fmsg_put(fmsg, "MAC", has_mac ? skb->mac_header : -1); > + devlink_fmsg_put(fmsg, "MAC len", > + has_mac ? skb_mac_header_len(skb) : -1); > + devlink_fmsg_put(fmsg, "network hdr", skb->network_header); > + devlink_fmsg_put(fmsg, "network hdr len", > + has_trans ? skb_network_header_len(skb) : -1); > + devlink_fmsg_put(fmsg, "transport hdr", > + has_trans ? skb->transport_header : -1); > + devlink_fmsg_put(fmsg, "csum", skb->csum); Hi, One minor nit here, which I don't think needs to stop progress of this patchset. Sparse warns that: error: no generic selection for 'restricted __wsum const [usertype] csum' I believe this can be addressed by casting: (__force __u32) skb->csum, perhaps incorporated into devlink_fmsg_put(). Which seems fine enough for this case. However, my observation is that there are a lot of sparse warnings present in the tree due to similar issues around the use of __wsum. And IMHO naked casts are error prone and not obviously correct to the reader (me). So I wonder if there is some value in introducing some helpers. E.g. wsum_to_cpu() cpu_to_wsum() To my mind, that would clearly be out of scope for this patchset. But It seems appropriate to raise this as it's been on my mind for a while. ...
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Przemek Kitszel > Sent: Wednesday, August 21, 2024 7:07 PM > To: intel-wired-lan@lists.osuosl.org; Jiri Pirko <jiri@resnulli.us>; Nguyen, Anthony L <anthony.l.nguyen@intel.com> > Cc: lukas.bulwahn@gmail.com; willemb@google.com; dwaipayanray1@gmail.com; netdev@vger.kernel.org; Polchlopek, Mateusz <mateusz.polchlopek@intel.com>; joe@perches.com; Eric Dumazet <edumazet@google.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Jakub Kicinski <kuba@kernel.org>; apw@canonical.com; NEX SW NCIS OSDT ITP Upstreaming <nex.sw.ncis.osdt.itp.upstreaming@intel.com>; akpm@linux-foundation.org; Paolo Abeni <pabeni@redhat.com>; David S. Miller <davem@davemloft.net> > Subject: [Intel-wired-lan] [PATCH iwl-next v3 3/6] devlink: add devlink_fmsg_dump_skb() function > > From: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > > Add devlink_fmsg_dump_skb() function that adds some diagnostic information about skb (like length, pkt type, MAC, etc) to devlink fmsg mechanism using bunch of devlink_fmsg_put() function calls. > > Signed-off-by: Mateusz Polchlopek <mateusz.polchlopek@intel.com> > Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@intel.com> > --- > include/net/devlink.h | 2 ++ > net/devlink/health.c | 67 +++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 69 insertions(+) > Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@intel.com> (A Contingent worker at Intel)
diff --git a/include/net/devlink.h b/include/net/devlink.h index 85739bb731c1..7f5b36554778 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1268,6 +1268,7 @@ enum devlink_trap_group_generic_id { u16 : devlink_fmsg_u32_pair_put, \ u32 : devlink_fmsg_u32_pair_put, \ u64 : devlink_fmsg_u64_pair_put, \ + int : devlink_fmsg_u32_pair_put, \ char * : devlink_fmsg_string_pair_put, \ const char * : devlink_fmsg_string_pair_put) \ (fmsg, name, (value))) @@ -2018,6 +2019,7 @@ int devlink_compat_switch_id_get(struct net_device *dev, int devlink_nl_port_handle_fill(struct sk_buff *msg, struct devlink_port *devlink_port); size_t devlink_nl_port_handle_size(struct devlink_port *devlink_port); +void devlink_fmsg_dump_skb(struct devlink_fmsg *fmsg, const struct sk_buff *skb); #else diff --git a/net/devlink/health.c b/net/devlink/health.c index acb8c0e174bb..b98ca650284c 100644 --- a/net/devlink/health.c +++ b/net/devlink/health.c @@ -1241,3 +1241,70 @@ int devlink_nl_health_reporter_test_doit(struct sk_buff *skb, return reporter->ops->test(reporter, info->extack); } + +/** + * devlink_fmsg_dump_skb - Dump sk_buffer structure + * @fmsg: devlink formatted message pointer + * @skb: pointer to skb + * + * Dump diagnostic information about sk_buff structure, like headroom, length, + * tailroom, MAC, etc. + */ +void devlink_fmsg_dump_skb(struct devlink_fmsg *fmsg, const struct sk_buff *skb) +{ + struct skb_shared_info *sh = skb_shinfo(skb); + struct sock *sk = skb->sk; + bool has_mac, has_trans; + + has_mac = skb_mac_header_was_set(skb); + has_trans = skb_transport_header_was_set(skb); + + devlink_fmsg_pair_nest_start(fmsg, "skb"); + devlink_fmsg_obj_nest_start(fmsg); + devlink_fmsg_put(fmsg, "actual len", skb->len); + devlink_fmsg_put(fmsg, "head len", skb_headlen(skb)); + devlink_fmsg_put(fmsg, "data len", skb->data_len); + devlink_fmsg_put(fmsg, "tail len", skb_tailroom(skb)); + devlink_fmsg_put(fmsg, "MAC", has_mac ? skb->mac_header : -1); + devlink_fmsg_put(fmsg, "MAC len", + has_mac ? skb_mac_header_len(skb) : -1); + devlink_fmsg_put(fmsg, "network hdr", skb->network_header); + devlink_fmsg_put(fmsg, "network hdr len", + has_trans ? skb_network_header_len(skb) : -1); + devlink_fmsg_put(fmsg, "transport hdr", + has_trans ? skb->transport_header : -1); + devlink_fmsg_put(fmsg, "csum", skb->csum); + devlink_fmsg_put(fmsg, "csum_ip_summed", (u8)skb->ip_summed); + devlink_fmsg_put(fmsg, "csum_complete_sw", !!skb->csum_complete_sw); + devlink_fmsg_put(fmsg, "csum_valid", !!skb->csum_valid); + devlink_fmsg_put(fmsg, "csum_level", (u8)skb->csum_level); + devlink_fmsg_put(fmsg, "sw_hash", !!skb->sw_hash); + devlink_fmsg_put(fmsg, "l4_hash", !!skb->l4_hash); + devlink_fmsg_put(fmsg, "proto", ntohs(skb->protocol)); + devlink_fmsg_put(fmsg, "pkt_type", (u8)skb->pkt_type); + devlink_fmsg_put(fmsg, "iif", skb->skb_iif); + + if (sk) { + devlink_fmsg_pair_nest_start(fmsg, "sk"); + devlink_fmsg_obj_nest_start(fmsg); + devlink_fmsg_put(fmsg, "family", sk->sk_type); + devlink_fmsg_put(fmsg, "type", sk->sk_type); + devlink_fmsg_put(fmsg, "proto", sk->sk_protocol); + devlink_fmsg_obj_nest_end(fmsg); + devlink_fmsg_pair_nest_end(fmsg); + } + + devlink_fmsg_obj_nest_end(fmsg); + devlink_fmsg_pair_nest_end(fmsg); + + devlink_fmsg_pair_nest_start(fmsg, "shinfo"); + devlink_fmsg_obj_nest_start(fmsg); + devlink_fmsg_put(fmsg, "tx_flags", sh->tx_flags); + devlink_fmsg_put(fmsg, "nr_frags", sh->nr_frags); + devlink_fmsg_put(fmsg, "gso_size", sh->gso_size); + devlink_fmsg_put(fmsg, "gso_type", sh->gso_type); + devlink_fmsg_put(fmsg, "gso_segs", sh->gso_segs); + devlink_fmsg_obj_nest_end(fmsg); + devlink_fmsg_pair_nest_end(fmsg); +} +EXPORT_SYMBOL_GPL(devlink_fmsg_dump_skb);