public inbox for netdev@vger.kernel.org 
 help / color / mirror / Atom feed
From: Nicolas Dichtel <nicolas.dichtel@6wind•com>
To: David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org, ebiederm@xmission•com, bcrl@kvack•org,
	ravi.mlists@gmail•com, bhutchings@solarflare•com,
	Eric Dumazet <eric.dumazet@gmail•com>
Subject: Re: [PATCH v2 net-next 2/2] sit: add support of x-netns
Date: Wed, 26 Jun 2013 15:49:07 +0200	[thread overview]
Message-ID: <51CAF153.4070809@6wind.com> (raw)
In-Reply-To: <20130625.165612.1653110297729408070.davem@davemloft.net>

Le 26/06/2013 01:56, David Miller a écrit :
> From: Nicolas Dichtel <nicolas.dichtel@6wind•com>
> Date: Tue, 25 Jun 2013 16:24:55 +0200
>
>> @@ -453,6 +454,8 @@ int ip_tunnel_rcv(struct ip_tunnel *tunnel, struct sk_buff *skb,
>>   	tstats->rx_bytes += skb->len;
>>   	u64_stats_update_end(&tstats->syncp);
>>
>> +	skb_scrub_packet(skb);
>> +
>>   	if (tunnel->dev->type == ARPHRD_ETHER) {
>>   		skb->protocol = eth_type_trans(skb, tunnel->dev);
>>   		skb_postpull_rcsum(skb, eth_hdr(skb), ETH_HLEN);
>
> I can't see how this can be ok.
>
> If something in netfilter depends upon the state you are clearing out
> here, someone's packet filtering setup is going to break.
Just for the record, note that nf_reset() is already called in 
iptunnel_pull_header() and iptunnel_xmit().
Hence 4in4 (ipip and sit) and gre tunnels are already reseting netfilter state.
6in4 (sit) do it only in xmit path and Xin6 (ip6_tunnel) never.

We can also notice that nf_reset() was added by the commit 3d7b46cd20e3 
"ip_tunnel: push generic protocol handling to ip_tunnel module." (net-next only) 
in rx path.

The nf_reset() of xmit path of 4in4 (ipip) is here for years (at least 2.6.12).
For gre, it has been added by c54419321455 "GRE: Refactor GRE tunneling code." 
(v3.10-rc1).

It seems that the code is different depending of the type of the tunnel. If we 
omit skb_orphan() (and maybe another one?, to be done only when changing 
namespace), it can be good to have a common function to have the same behavior 
for each tunnel.

Maybe something like:
void skb_scrub_packet(bool netnschange)
{
	if (netnschange)
	        skb_orphan(skb);
         skb->tstamp.tv64 = 0;
         skb->pkt_type = PACKET_HOST;
         skb->skb_iif = 0;
         skb_dst_drop(skb);
         skb->mark = 0;
         secpath_reset(skb);
         nf_reset(skb);
         nf_reset_trace(skb);
}

What's your opinion?

      parent reply	other threads:[~2013-06-26 13:49 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-23 17:49 switching network namespace midway rsa
2012-10-24 21:11 ` Eric W. Biederman
2012-10-24 21:21   ` Benjamin LaHaise
2012-10-25  1:37     ` Eric W. Biederman
2012-10-25 14:38       ` Benjamin LaHaise
2012-10-25 16:21         ` Stephen Hemminger
2012-10-28  5:43           ` Eric W. Biederman
2012-10-29 14:23             ` Stephen Hemminger
2012-10-30  0:21               ` Eric W. Biederman
2012-10-30  8:55                 ` James Chapman
2012-10-25 15:12     ` rsa
2012-10-25 15:29     ` rsa
2012-10-25 15:59       ` Benjamin LaHaise
2012-10-25 16:15         ` Eric W. Biederman
2012-11-02  2:25           ` Benjamin LaHaise
2012-11-02  6:18             ` Eric W. Biederman
2012-11-02 14:03               ` Benjamin LaHaise
2012-11-02 20:45                 ` Eric W. Biederman
2013-06-24 14:13                   ` [RFC PATCH net-next 0/2] sit: allow to switch netns during encap/decap Nicolas Dichtel
2013-06-24 14:13                     ` [RFC PATCH net-next 1/2] dev: introduce dev_cleanup_skb() Nicolas Dichtel
2013-06-24 18:13                       ` Ben Hutchings
2013-06-24 19:05                         ` Eric W. Biederman
2013-06-24 14:13                     ` [RFC PATCH net-next 2/2] sit: add support of x-netns Nicolas Dichtel
2013-06-24 19:28                       ` Eric W. Biederman
2013-06-24 21:11                         ` Nicolas Dichtel
2013-06-24 22:42                           ` Eric W. Biederman
2013-06-25 14:10                             ` Nicolas Dichtel
2013-06-25 14:24                               ` [PATCH v2 net-next 0/2] sit: allow to switch netns during encap/decap Nicolas Dichtel
2013-06-25 14:24                                 ` [PATCH v2 net-next 1/2] dev: introduce skb_scrub_packet() Nicolas Dichtel
2013-06-25 14:24                                 ` [PATCH v2 net-next 2/2] sit: add support of x-netns Nicolas Dichtel
2013-06-25 23:56                                   ` David Miller
2013-06-26  1:35                                     ` Eric W. Biederman
2013-06-26  5:48                                       ` David Miller
2013-06-26 10:03                                         ` Eric W. Biederman
2013-06-26 10:22                                           ` Eric Dumazet
2013-06-26 12:15                                             ` Nicolas Dichtel
2013-06-26 14:11                                               ` [PATCH v3 net-next 0/2] sit: allow to switch netns during encap/decap Nicolas Dichtel
2013-06-26 14:11                                                 ` [PATCH v3 net-next 1/2] dev: introduce skb_scrub_packet() Nicolas Dichtel
2013-06-26 14:11                                                 ` [PATCH v3 net-next 2/2] sit: add support of x-netns Nicolas Dichtel
2013-06-28  5:36                                                 ` [PATCH v3 net-next 0/2] sit: allow to switch netns during encap/decap David Miller
2013-07-03 15:00                                                   ` [PATCH net-next 0/3] ipip/ip6tnl: " Nicolas Dichtel
2013-07-03 15:00                                                     ` [PATCH net-next 1/3] sit: fix tunnel update via netlink Nicolas Dichtel
2013-07-03 15:00                                                     ` [PATCH net-next 2/3] ipip: add x-netns support Nicolas Dichtel
2013-07-03 15:00                                                     ` [PATCH net-next 3/3] ip6tnl: " Nicolas Dichtel
2013-07-04 21:56                                                     ` [PATCH net-next 0/3] ipip/ip6tnl: allow to switch netns during encap/decap David Miller
2013-08-13 15:51                                                       ` [PATCH net-next v2 0/4] " Nicolas Dichtel
2013-08-13 15:51                                                         ` [PATCH net-next v2 1/4] dev: move skb_scrub_packet() after eth_type_trans() Nicolas Dichtel
2013-08-13 15:51                                                         ` [PATCH net-next v2 2/4] ipv4 tunnels: use net_eq() helper to check netns Nicolas Dichtel
2013-08-13 15:51                                                         ` [PATCH net-next v2 3/4] ipip: add x-netns support Nicolas Dichtel
2013-08-13 15:51                                                         ` [PATCH net-next v2 4/4] ip6tnl: " Nicolas Dichtel
2013-08-15  8:01                                                         ` [PATCH net-next v2 0/4] ipip/ip6tnl: allow to switch netns during encap/decap David Miller
2013-06-26 13:49                                     ` Nicolas Dichtel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=51CAF153.4070809@6wind.com \
    --to=nicolas.dichtel@6wind$(echo .)com \
    --cc=bcrl@kvack$(echo .)org \
    --cc=bhutchings@solarflare$(echo .)com \
    --cc=davem@davemloft$(echo .)net \
    --cc=ebiederm@xmission$(echo .)com \
    --cc=eric.dumazet@gmail$(echo .)com \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=ravi.mlists@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox