diff mbox

net: ipv6: Disable forwarding per interface via sysctl

Message ID 1474030076-24809-1-git-send-email-mmanning@brocade.com
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Mike Manning Sept. 16, 2016, 12:47 p.m. UTC
Disabling forwarding per interface via sysctl continues to allow
forwarding. This is contrary to the sysctl documentation stating that
the forwarding sysctl is per interface, whereas currently it is only
the sysctl for all interfaces that has an effect on forwarding. The
solution is to drop any received packets instead of forwarding them
if the ingress device has a per-device forwarding sysctl that is unset.

Signed-off-by: Mike Manning <mmanning@brocade.com>
---
 net/ipv6/ip6_output.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Eric Dumazet Sept. 16, 2016, 1:39 p.m. UTC | #1
On Fri, 2016-09-16 at 13:47 +0100, Mike Manning wrote:
> Disabling forwarding per interface via sysctl continues to allow
> forwarding. This is contrary to the sysctl documentation stating that
> the forwarding sysctl is per interface, whereas currently it is only
> the sysctl for all interfaces that has an effect on forwarding. The
> solution is to drop any received packets instead of forwarding them
> if the ingress device has a per-device forwarding sysctl that is unset.

Some archaeological research might be needed because
Documentation/networking/ip-sysctl.txt states :

IPv4 and IPv6 work differently here; e.g. netfilter must be used
to control which interfaces may forward packets and which not.

If this netfilter requirement is obsolete, then your patch would need to
change the doc as well.

Hannes can probably comment on this ?

Thanks.
Hannes Frederic Sowa Sept. 16, 2016, 3:46 p.m. UTC | #2
On 16.09.2016 15:39, Eric Dumazet wrote:
> On Fri, 2016-09-16 at 13:47 +0100, Mike Manning wrote:
>> Disabling forwarding per interface via sysctl continues to allow
>> forwarding. This is contrary to the sysctl documentation stating that
>> the forwarding sysctl is per interface, whereas currently it is only
>> the sysctl for all interfaces that has an effect on forwarding. The
>> solution is to drop any received packets instead of forwarding them
>> if the ingress device has a per-device forwarding sysctl that is unset.
> 
> Some archaeological research might be needed because
> Documentation/networking/ip-sysctl.txt states :
> 
> IPv4 and IPv6 work differently here; e.g. netfilter must be used
> to control which interfaces may forward packets and which not.
> 
> If this netfilter requirement is obsolete, then your patch would need to
> change the doc as well.
> 
> Hannes can probably comment on this ?

Yep, thanks.

This commit breaks a very common setup: people globally enabled
forwarding but disabled the forwarding knob on one special interface to
allow this interface to participate in auto configuration from their
provider while still forwarding packets over this interface.

I fear this is so common that this would be a uapi violation.

Thanks,
Hannes
Mike Manning Sept. 16, 2016, 4:25 p.m. UTC | #3
On 09/16/2016 04:46 PM, Hannes Frederic Sowa wrote:
> On 16.09.2016 15:39, Eric Dumazet wrote:
>> On Fri, 2016-09-16 at 13:47 +0100, Mike Manning wrote:
>>> Disabling forwarding per interface via sysctl continues to allow
>>> forwarding. This is contrary to the sysctl documentation stating that
>>> the forwarding sysctl is per interface, whereas currently it is only
>>> the sysctl for all interfaces that has an effect on forwarding. The
>>> solution is to drop any received packets instead of forwarding them
>>> if the ingress device has a per-device forwarding sysctl that is unset.
>>
>> Some archaeological research might be needed because
>> Documentation/networking/ip-sysctl.txt states :
>>
>> IPv4 and IPv6 work differently here; e.g. netfilter must be used
>> to control which interfaces may forward packets and which not.
>>
>> If this netfilter requirement is obsolete, then your patch would need to
>> change the doc as well.
>>
>> Hannes can probably comment on this ?
> 
> Yep, thanks.
> 
> This commit breaks a very common setup: people globally enabled
> forwarding but disabled the forwarding knob on one special interface to
> allow this interface to participate in auto configuration from their
> provider while still forwarding packets over this interface.
> 
> I fear this is so common that this would be a uapi violation.
> 
> Thanks,
> Hannes
> 
> 
Thanks for the use-case, I request to withdraw this patch then.
So configuring an interface on a router to be in host mode is not actually
disabling forwarding in the kernel, it is merely to allow SLAAC. Using ip6tables
for the purpose of disabling forwarding on an interface if one wants an interface
in host mode seems a heavyweight solution to work around this. If anyone has
any better suggestions, please let me know.
diff mbox

Patch

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 1dfc402..37cd1d0 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -380,11 +380,15 @@  int ip6_forward(struct sk_buff *skb)
 	struct ipv6hdr *hdr = ipv6_hdr(skb);
 	struct inet6_skb_parm *opt = IP6CB(skb);
 	struct net *net = dev_net(dst->dev);
+	struct inet6_dev *idev = __in6_dev_get(skb->dev);
 	u32 mtu;
 
 	if (net->ipv6.devconf_all->forwarding == 0)
 		goto error;
 
+	if (idev && !idev->cnf.forwarding)
+		goto error;
+
 	if (skb->pkt_type != PACKET_HOST)
 		goto drop;