> -----Original Message----- > From: netfilter-devel-owner@xxxxxxxxxxxxxxx > On Fri, Apr 14, 2017 at 07:04:44AM +0800, Gao Feng wrote: > > > -----Original Message----- > > > From: Pablo Neira Ayuso [mailto:pablo@xxxxxxxxxxxxx] > > > > > > On Wed, Apr 12, 2017 at 10:14:50AM +0800, gfree.wind@xxxxxxxxxxx > wrote: > > > > > > > > Current SYNPROXY codes return NF_DROP during normal TCP > > > > handshaking, it is not friendly to caller. Because the > > > > nf_hook_slow would treat the NF_DROP as an error, and return -EPERM. > > > > As a result, it may cause the top caller think it meets one error. > > > > > > > > So use NF_STOLEN instead of NF_DROP now because there is no error > > > > happened indeed, and free the skb directly. > > > > > > Is this really addressing a real problem? How did you reproduce it? > > > > We defined the NF_DROP and NF_STOLEN, I think we should use them > clearly. > > When NF_DROP happens, it means one error happened. > > That's a valid concern. How did you tested this change? The test is a little hacker. The following is my whole test process. 1. Add one "print" member in the struct sk_buff; it would be zero by default; 2. Add one log in the netif_receive_skb_internal + if (skb->print) + pr_info("skb(%p) ret is %d\n", skb, ret); 3. the iptable rule is "iptables -A INPUT -p tcp --dport 12345 -m conntrack --ctstate NEW -j SYNPROXY" Test NF_DROP with the original codes: 1. I comment out the "kfree_skb" in the NF_DROP handler of nf_hook_slow. 2. Add one log before return NF_DROP in the synproxy codes pr_info("skb(%p) is dropped\n", skb); The result is following: [ 71.765035] skb(ffff9a6208bd7500) is dropped [ 71.765049] skb(ffff9a6208bd7500) ret is -1 Test NF_STOLEN with the patch: 1. I comment out the "consume_skb" before return NF_STOLEN; 2. Add the log before return NF_STOLEN in the synproxy codes pr_info("skb(%p) is stolen\n", skb); The test result is following: [ 221.564370] skb(ffff9a01214a8c00) is stolen [ 221.564383] skb(ffff9a01214a8c00) ret is 0 To summary, netif_receive_skb would return -EPERM when Netfilter returns NF_DROP, but NF_STOLEN not. For the caller which cares about the return value of netif_receive_skb would treat it as one error. Like cfv_rx_poll() in drivers/net/caif/caif_virtio.c. err = netif_receive_skb(skb); if (unlikely(err)) { ++cfv->ndev->stats.rx_dropped; } else { ++cfv->ndev->stats.rx_packets; cfv->ndev->stats.rx_bytes += skb_len; } It would cause the driver increase the dropped counter. Best Regards Feng > > [...] > > Sorry, I always use one command "git format-patch -s -n master..XX" > > according to one document > > whose title is "HOWTO: Create and submit your first Linux kernel patch > > using GIT". > > > > It generate the "1/1" by default. > > There are ways to avoid that. Only you send 1/1 patches. > > > I will try to lookup other documents about the patch rule, and correct > > the current command. > > OK. > > [...] > > More carefully, and don't rush more. > > Thank you. > -- > To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the > body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at > http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
