On 3/21/18 8:26 AM, Eric Dumazet wrote:
On 03/20/2018 11:47 PM, Yonghong Song wrote:
+static __init int test_skb_segment(void)
+{
+ netdev_features_t features;
+ struct sk_buff *skb;
+ int ret = -1;
+
+ features = NETIF_F_SG | NETIF_F_GSO_PARTIAL | NETIF_F_IP_CSUM |
+ NETIF_F_IPV6_CSUM;
+ features |= NETIF_F_RXCSUM;
+ skb = build_test_skb();
+ if (!skb) {
+ pr_info("%s: failed to build_test_skb", __func__);
+ goto done;
+ }
+
+ if (skb_segment(skb, features)) {
+ ret = 0;
+ pr_info("%s: success in skb_segment!", __func__);
+ } else {
+ pr_info("%s: failed in skb_segment!", __func__);
+ }
+ kfree_skb(skb);
If skb_segmen() was successful (original) skb was already freed.
kfree_skb(old_skb) should thus panic the box, if you run this code
on a kernel having some debugging features like KASAN
I tried with KASAN. It does not panic.
Looking at the code in net/core/dev.c: validate_xmit_skb:
static struct sk_buff *validate_xmit_skb(struct sk_buff *skb, struct
net_device *dev, bool *again)
...
if (netif_needs_gso(skb, features)) {
struct sk_buff *segs;
segs = skb_gso_segment(skb, features);
if (IS_ERR(segs)) {
goto out_kfree_skb;
} else if (segs) {
consume_skb(skb);
skb = segs;
}
...
out_kfree_skb:
kfree_skb(skb);
which also indicates kfree_skb/consume_skb probably is the right way
to free skb after skb_gso_segment/skb_segment.
This probably explains why my above kfree_skb(skb) does not crash.
So you must store in a variable the return of skb_segment(),
to be able to free skb(s), using kfree_skb_list()
Totally agree. Will make the change. Thanks!
+done:
+ return ret;
+}
+
