Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Liran Alon
On 20/03/18 20:51, valdis.kletni...@vt.edu wrote: On Tue, 20 Mar 2018 18:39:47 +0200, Liran Alon said: What is your opinion in regards if it's OK to put the flag enabling this "fix" in /proc/sys/net/core? Do you think it's sufficient? Umm.. *which* /proc/sys/net/core? These could differ

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread valdis . kletnieks
On Tue, 20 Mar 2018 18:39:47 +0200, Liran Alon said: > What is your opinion in regards if it's OK to put the flag enabling this > "fix" in /proc/sys/net/core? Do you think it's sufficient? Umm.. *which* /proc/sys/net/core? These could differ for things that are in different namespaces. Or are

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Eric W. Biederman
Ben Greear writes: > On 03/20/2018 09:44 AM, Liran Alon wrote: >> >> >> On 20/03/18 18:24, ebied...@xmission.com wrote: >>> >>> I don't believe the current behavior is a bug. >>> >>> I looked through the history. Basically skb_scrub_packet >>> started out as the

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Ben Greear
On 03/20/2018 09:44 AM, Liran Alon wrote: On 20/03/18 18:24, ebied...@xmission.com wrote: I don't believe the current behavior is a bug. I looked through the history. Basically skb_scrub_packet started out as the scrubbing needed for crossing network namespaces. Then tunnels which needed

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Liran Alon
On 20/03/18 18:24, ebied...@xmission.com wrote: I don't believe the current behavior is a bug. I looked through the history. Basically skb_scrub_packet started out as the scrubbing needed for crossing network namespaces. Then tunnels which needed 90% of the functionality started calling

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Liran Alon
On 20/03/18 18:34, David Miller wrote: From: Liran Alon Date: Tue, 20 Mar 2018 18:11:49 +0200 1. Do we want to make a flag for every bug that is user-space visible? I think there is place for consideration on a per-case basis. I still don't see how a user can utilize

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread David Miller
From: Liran Alon Date: Tue, 20 Mar 2018 18:11:49 +0200 > 1. Do we want to make a flag for every bug that is user-space visible? > I think there is place for consideration on a per-case basis. I still > don't see how a user can utilize this behaviour. He is basically >

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Eric W. Biederman
Liran Alon writes: > - shmulik.ladk...@gmail.com wrote: > >> On Thu, 15 Mar 2018 09:35:51 -0700 (PDT) Liran Alon >> wrote: >> > - shmulik.ladk...@gmail.com wrote: >> > >> > > On Thu, 15 Mar 2018 08:01:03 -0700 (PDT) Liran Alon >> > >

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Liran Alon
On 20/03/18 18:00, David Miller wrote: From: Liran Alon Date: Tue, 20 Mar 2018 17:34:38 +0200 I personally don't understand why we should maintain backwards-comparability to this behaviour. The reason is because not breaking things is a cornerstone of Linux kernel

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread David Miller
From: Liran Alon Date: Tue, 20 Mar 2018 17:34:38 +0200 > I personally don't understand why we should maintain > backwards-comparability to this behaviour. The reason is because not breaking things is a cornerstone of Linux kernel development. > This feature is not

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread Liran Alon
On 20/03/18 16:47, David Miller wrote: From: Liran Alon Date: Tue, 13 Mar 2018 17:07:22 +0200 Before this commit, dev_forward_skb() always cleared packet's per-network-namespace info. Even if the packet doesn't cross network namespaces. There was a lot of discussion

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-20 Thread David Miller
From: Liran Alon Date: Tue, 13 Mar 2018 17:07:22 +0200 > Before this commit, dev_forward_skb() always cleared packet's > per-network-namespace info. Even if the packet doesn't cross > network namespaces. There was a lot of discussion about this patch. Particularly

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Daniel Borkmann
On 03/15/2018 04:54 PM, Shmulik Ladkani wrote: > On Thu, 15 Mar 2018 16:13:39 +0100 Daniel Borkmann > wrote: >> On 03/15/2018 01:50 PM, Shmulik Ladkani wrote: >>> >>> It would be beneficial to have the mark preserved when skb is injected >>> to the slave device's rx path

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- shmulik.ladk...@gmail.com wrote: > On Thu, 15 Mar 2018 09:35:51 -0700 (PDT) Liran Alon > wrote: > > - shmulik.ladk...@gmail.com wrote: > > > > > On Thu, 15 Mar 2018 08:01:03 -0700 (PDT) Liran Alon > > > wrote: > > > > > > > > I still

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Shmulik Ladkani
On Thu, 15 Mar 2018 09:35:51 -0700 (PDT) Liran Alon wrote: > - shmulik.ladk...@gmail.com wrote: > > > On Thu, 15 Mar 2018 08:01:03 -0700 (PDT) Liran Alon > > wrote: > > > > > > I still think that default behavior should be to zero skb->mark

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- shmulik.ladk...@gmail.com wrote: > On Thu, 15 Mar 2018 08:01:03 -0700 (PDT) Liran Alon > wrote: > > > > I still think that default behavior should be to zero skb->mark only > when skb > > cross netdevs in different netns. > > But the previous default was scrub the

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Shmulik Ladkani
On Thu, 15 Mar 2018 08:01:03 -0700 (PDT) Liran Alon wrote: > > I still think that default behavior should be to zero skb->mark only when skb > cross netdevs in different netns. But the previous default was scrub the mark in *both* xnet and non-xnet situations.

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Shmulik Ladkani
On Thu, 15 Mar 2018 16:13:39 +0100 Daniel Borkmann wrote: > On 03/15/2018 01:50 PM, Shmulik Ladkani wrote: > > > > It would be beneficial to have the mark preserved when skb is injected > > to the slave device's rx path (especially when it's on the same netns). > >

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Daniel Borkmann
On 03/15/2018 01:50 PM, Shmulik Ladkani wrote: > On Thu, 15 Mar 2018 12:56:13 +0100 Daniel Borkmann > wrote: >> On 03/15/2018 10:21 AM, Shmulik Ladkani wrote: >>> >>> Regarding veth xmit, it does makes sense to preserve the fields if not >>> crossing netns. This is also the

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- dan...@iogearbox.net wrote: > On 03/15/2018 03:35 PM, Roman Mashak wrote: > > Liran Alon writes: > > [...] > >>> Overall I think it might be nice to not need scrubbing skb in > such > >>> cases, > >>> although my concern would be that this has potential to break >

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- m...@mojatatu.com wrote: > Liran Alon writes: > > > [...] > > >> Overall I think it might be nice to not need scrubbing skb in such > >> cases, > >> although my concern would be that this has potential to break > >> existing > >> setups when they would expect

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Daniel Borkmann
On 03/15/2018 03:35 PM, Roman Mashak wrote: > Liran Alon writes: > [...] >>> Overall I think it might be nice to not need scrubbing skb in such >>> cases, >>> although my concern would be that this has potential to break >>> existing >>> setups when they would expect mark

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Roman Mashak
Liran Alon writes: [...] >> Overall I think it might be nice to not need scrubbing skb in such >> cases, >> although my concern would be that this has potential to break >> existing >> setups when they would expect mark being zero on other veth peer in >> any >> case

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Shmulik Ladkani
Hi, On Thu, 15 Mar 2018 12:56:13 +0100 Daniel Borkmann wrote: > On 03/15/2018 10:21 AM, Shmulik Ladkani wrote: > > > > Regarding veth xmit, it does makes sense to preserve the fields if not > > crossing netns. This is also the case when one uses tc mirred. > > > >

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- dan...@iogearbox.net wrote: > On 03/15/2018 10:21 AM, Shmulik Ladkani wrote: > > Regarding the premise of this commit, this "reduces" the > > ipvs/orphan/mark scrubbing in the following *non* xnet situations: > > > > 1. mac2vlan port xmit to other macvlan ports in Bridge Mode > > 2.

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Liran Alon
- shmulik.ladk...@gmail.com wrote: > Hi, > > On Tue, 13 Mar 2018 17:07:22 +0200 Liran Alon > wrote: > > Before this commit, dev_forward_skb() always cleared packet's > > per-network-namespace info. Even if the packet doesn't cross > > network namespaces. > > > > The

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Daniel Borkmann
On 03/15/2018 10:21 AM, Shmulik Ladkani wrote: > Hi, > > On Tue, 13 Mar 2018 17:07:22 +0200 Liran Alon wrote: >> Before this commit, dev_forward_skb() always cleared packet's >> per-network-namespace info. Even if the packet doesn't cross >> network namespaces. >> >> The

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-15 Thread Shmulik Ladkani
Hi, On Tue, 13 Mar 2018 17:07:22 +0200 Liran Alon wrote: > Before this commit, dev_forward_skb() always cleared packet's > per-network-namespace info. Even if the packet doesn't cross > network namespaces. > > The comment above dev_forward_skb() describes that this is

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-14 Thread Yuval Shaia
On Tue, Mar 13, 2018 at 06:13:45PM +0200, Yuval Shaia wrote: > On Tue, Mar 13, 2018 at 05:07:22PM +0200, Liran Alon wrote: > > Before this commit, dev_forward_skb() always cleared packet's > > per-network-namespace info. Even if the packet doesn't cross > > network namespaces. > > > > The comment

Re: [PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-13 Thread Yuval Shaia
On Tue, Mar 13, 2018 at 05:07:22PM +0200, Liran Alon wrote: > Before this commit, dev_forward_skb() always cleared packet's > per-network-namespace info. Even if the packet doesn't cross > network namespaces. > > The comment above dev_forward_skb() describes that this is done > because the

[PATCH] net: dev_forward_skb(): Scrub packet's per-netns info only when crossing netns

2018-03-13 Thread Liran Alon
Before this commit, dev_forward_skb() always cleared packet's per-network-namespace info. Even if the packet doesn't cross network namespaces. The comment above dev_forward_skb() describes that this is done because the receiving device may be in another network namespace. However, this case can