On 7/14/2024 4:30 PM, Jakub Kicinski wrote:
On Fri, 12 Jul 2024 05:32:51 -0400 Mateusz Polchlopek wrote:
+       buf_pos = ice_emit_to_buf(buf, buf_size, buf_pos,
+               "skb len=%u headroom=%u headlen=%u tailroom=%u\n"
+               "mac=(%d,%d) net=(%d,%d) trans=%d\n"
+               "shinfo(txflags=%u nr_frags=%u gso(size=%hu type=%u 
segs=%hu))\n"
+               "csum(0x%x ip_summed=%u complete_sw=%u valid=%u level=%u)\n"
+               "hash(0x%x sw=%u l4=%u) proto=0x%04x pkttype=%u iif=%d\n",
+               skb->len, headroom, skb_headlen(skb), tailroom,
+               has_mac ? skb->mac_header : -1,
+               has_mac ? skb_mac_header_len(skb) : -1,
+               skb->network_header,
+               has_trans ? skb_network_header_len(skb) : -1,
+               has_trans ? skb->transport_header : -1,
+               sh->tx_flags, sh->nr_frags,
+               sh->gso_size, sh->gso_type, sh->gso_segs,
+               skb->csum, skb->ip_summed, skb->csum_complete_sw,
+               skb->csum_valid, skb->csum_level,
+               skb->hash, skb->sw_hash, skb->l4_hash,
+               ntohs(skb->protocol), skb->pkt_type, skb->skb_iif);

Make it a generic helper in devlink?

+       if (dev)
+               buf_pos = ice_emit_to_buf(buf, buf_size, buf_pos,
+                                         "dev name=%s feat=%pNF\n", dev->name,
+                                         &dev->features);
+       if (sk)
+               buf_pos = ice_emit_to_buf(buf, buf_size, buf_pos,
+                                         "sk family=%hu type=%u proto=%u\n",
+                                         sk->sk_family, sk->sk_type,
+                                         sk->sk_protocol);
+
+       if (headroom)
+               buf_pos = ice_emit_hex_to_buf(buf, buf_size, buf_pos,
+                                             "skb headroom: ", skb->head,
+                                             headroom);
+
+       seg_len = min_t(int, skb_headlen(skb), len);
+       if (seg_len)
+               buf_pos = ice_emit_hex_to_buf(buf, buf_size, buf_pos,
+                                             "skb linear:   ", skb->data,
+                                             seg_len);
+       len -= seg_len;
+
+       if (tailroom)
+               buf_pos = ice_emit_hex_to_buf(buf, buf_size, buf_pos,
+                                             "skb tailroom: ",
+                                             skb_tail_pointer(skb), tailroom);

The printing on tailroom, headroom and frag data seems a bit much.
I guess you're only printing the head SKB so it may be fine. But
I don't think it's useful. The device will probably only care about
the contents of the headers, for other parts only the metadata matters.
No strong preference tho.
 >> +     for (i = 0; len && i < skb_shinfo(skb)->nr_frags; i++) {
+               skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+               u32 p_off, p_len, copied;
+               struct page *p;
+               u8 *vaddr;
+
+               skb_frag_foreach_page(frag, skb_frag_off(frag),
+                                     skb_frag_size(frag), p, p_off, p_len,
+                                     copied) {
+                       seg_len = min_t(int, p_len, len);
+                       vaddr = kmap_local_page(p);
+                       buf_pos = ice_emit_hex_to_buf(buf, buf_size, buf_pos,
+                                                     "skb frag:     ",
+                                                     vaddr + p_off, seg_len);
+                       kunmap_local(vaddr);
+                       len -= seg_len;
+
+                       if (!len || buf_pos == buf_size)
+                               break;
+               }
+       }
+
+       if (skb_has_frag_list(skb)) {
+               buf_pos = ice_emit_to_buf(buf, buf_size, buf_pos,
+                                         "skb fraglist:\n");
+               skb_walk_frags(skb, list_skb) {
+                       buf_pos = ice_skb_dump_buf(buf, buf_size, buf_pos,
+                                                  list_skb);
+
+                       if (buf_pos == buf_size)
+                               break;
+               }
+       }

You support transmitting skbs with fraglist? 🤨️

This will be removed in the next version, we will log only the number of
frags

Reply via email to