Re: [PATCH net-next v2 1/1] net: Allow a rule to track originating protocol

2018-02-22 Thread David Ahern 
On 2/22/18 10:20 AM, David Ahern wrote:
>> This breaks my scripts:
>> # ip -4 rule show
>> 0:  from all lookup local
>> 32766:  from all lookup main
>> 32767:  from all lookup default
>>
>> # ip -4 rule del pref 0
>> RTNETLINK answers: No such file or directory
>>
>> Using iproute 4.15 in Fedora 27:
>> # ip -V
>> ip utility, iproute2-ss180129
>>
>> Problem is iproute sets protocol to RTPROT_BOOT while rules are
>> installed with RTPROT_KERNEL.
>>
>> Maybe add FRA_PROTOCOL?
>>
>> Thanks!
> 
> ugh. Another iproute2 bug that the kernel has to deal with. iproute2 has
> been using rtm for the ancillary header for rules when it should have
> been fib_rule_hdr. That bug allowed someone to set the protocol field to
> RTPROT_BOOT which was complete nonsense for rules until Donald's recent
> patch.
> 
> That means all FIB rules need to default to RTPROT_BOOT. I hate to
> inherit that for the l3mdev rule, but looking at the iproute2 code I
> don't see any options.
> 
> Donald: send a patch that changes the protocol for kernel installed
> rules to RTPROT_BOOT.
> 

After more thinking, Donald is going move the protocol to an
FRA_PROTOCOL attribute as you suggested. That avoids breaking legacy
iproute2 and allows us to keep a sane default protocol value.

Re: [PATCH net-next v2 1/1] net: Allow a rule to track originating protocol

2018-02-22 Thread David Ahern 
On 2/22/18 1:23 AM, Ido Schimmel wrote:
>> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
>> index 98e1066c3d55..c1d4ab5b2d9f 100644
>> --- a/net/core/fib_rules.c
>> +++ b/net/core/fib_rules.c
>> @@ -51,6 +51,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
>>  r->pref = pref;
>>  r->table = table;
>>  r->flags = flags;
>> +r->proto = RTPROT_KERNEL;
>>  r->fr_net = ops->fro_net;
>>  r->uid_range = fib_kuid_range_unset;
>>  
>> @@ -465,6 +466,7 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr 
>> *nlh,
>>  }
>>  refcount_set(>refcnt, 1);
>>  rule->fr_net = net;
>> +rule->proto = frh->proto;
>>  
>>  rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
>>: fib_default_rule_pref(ops);
>> @@ -664,6 +666,9 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr 
>> *nlh,
>>  }
>>  
>>  list_for_each_entry(rule, >rules_list, list) {
>> +if (frh->proto && (frh->proto != rule->proto))
>> +continue;
> 
> This breaks my scripts:
> # ip -4 rule show
> 0:  from all lookup local
> 32766:  from all lookup main
> 32767:  from all lookup default
> 
> # ip -4 rule del pref 0
> RTNETLINK answers: No such file or directory
> 
> Using iproute 4.15 in Fedora 27:
> # ip -V
> ip utility, iproute2-ss180129
> 
> Problem is iproute sets protocol to RTPROT_BOOT while rules are
> installed with RTPROT_KERNEL.
> 
> Maybe add FRA_PROTOCOL?
> 
> Thanks!

ugh. Another iproute2 bug that the kernel has to deal with. iproute2 has
been using rtm for the ancillary header for rules when it should have
been fib_rule_hdr. That bug allowed someone to set the protocol field to
RTPROT_BOOT which was complete nonsense for rules until Donald's recent
patch.

That means all FIB rules need to default to RTPROT_BOOT. I hate to
inherit that for the l3mdev rule, but looking at the iproute2 code I
don't see any options.

Donald: send a patch that changes the protocol for kernel installed
rules to RTPROT_BOOT.

Re: [PATCH net-next v2 1/1] net: Allow a rule to track originating protocol

2018-02-22 Thread Ido Schimmel 
Hi Donald,

On Tue, Feb 20, 2018 at 08:55:58AM -0500, Donald Sharp wrote:
> Allow a rule that is being added/deleted/modified or
> dumped to contain the originating protocol's id.
> 
> The protocol is handled just like a routes originating
> protocol is.  This is especially useful because there
> is starting to be a plethora of different user space
> programs adding rules.
> 
> Allow the vrf device to specify that the kernel is the originator
> of the rule created for this device.
> 
> Signed-off-by: Donald Sharp 
> ---
>  drivers/net/vrf.c  | 1 +
>  include/net/fib_rules.h| 3 ++-
>  include/uapi/linux/fib_rules.h | 2 +-
>  net/core/fib_rules.c   | 7 ++-
>  4 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index 139c61c8244a..ec6d2d623b60 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -1175,6 +1175,7 @@ static int vrf_fib_rule(const struct net_device *dev, 
> __u8 family, bool add_it)
>   memset(frh, 0, sizeof(*frh));
>   frh->family = family;
>   frh->action = FR_ACT_TO_TBL;
> + frh->proto = RTPROT_KERNEL;
>  
>   if (nla_put_u8(skb, FRA_L3MDEV, 1))
>   goto nla_put_failure;
> diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
> index 648caf90ec07..b166ef07e6d4 100644
> --- a/include/net/fib_rules.h
> +++ b/include/net/fib_rules.h
> @@ -26,7 +26,8 @@ struct fib_rule {
>   u32 table;
>   u8  action;
>   u8  l3mdev;
> - /* 2 bytes hole, try to use */
> + u8  proto;
> + /* 1 byte hole, try to use */
>   u32 target;
>   __be64  tun_id;
>   struct fib_rule __rcu   *ctarget;
> diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
> index 2b642bf9b5a0..925539172d5b 100644
> --- a/include/uapi/linux/fib_rules.h
> +++ b/include/uapi/linux/fib_rules.h
> @@ -23,8 +23,8 @@ struct fib_rule_hdr {
>   __u8tos;
>  
>   __u8table;
> + __u8proto;
>   __u8res1;   /* reserved */
> - __u8res2;   /* reserved */
>   __u8action;
>  
>   __u32   flags;
> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
> index 98e1066c3d55..c1d4ab5b2d9f 100644
> --- a/net/core/fib_rules.c
> +++ b/net/core/fib_rules.c
> @@ -51,6 +51,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
>   r->pref = pref;
>   r->table = table;
>   r->flags = flags;
> + r->proto = RTPROT_KERNEL;
>   r->fr_net = ops->fro_net;
>   r->uid_range = fib_kuid_range_unset;
>  
> @@ -465,6 +466,7 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr 
> *nlh,
>   }
>   refcount_set(>refcnt, 1);
>   rule->fr_net = net;
> + rule->proto = frh->proto;
>  
>   rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
> : fib_default_rule_pref(ops);
> @@ -664,6 +666,9 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr 
> *nlh,
>   }
>  
>   list_for_each_entry(rule, >rules_list, list) {
> + if (frh->proto && (frh->proto != rule->proto))
> + continue;

This breaks my scripts:
# ip -4 rule show
0:  from all lookup local
32766:  from all lookup main
32767:  from all lookup default

# ip -4 rule del pref 0
RTNETLINK answers: No such file or directory

Using iproute 4.15 in Fedora 27:
# ip -V
ip utility, iproute2-ss180129

Problem is iproute sets protocol to RTPROT_BOOT while rules are
installed with RTPROT_KERNEL.

Maybe add FRA_PROTOCOL?

Thanks!

> +
>   if (frh->action && (frh->action != rule->action))
>   continue;
>  
> @@ -808,9 +813,9 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct 
> fib_rule *rule,
>   if (nla_put_u32(skb, FRA_SUPPRESS_PREFIXLEN, rule->suppress_prefixlen))
>   goto nla_put_failure;
>   frh->res1 = 0;
> - frh->res2 = 0;
>   frh->action = rule->action;
>   frh->flags = rule->flags;
> + frh->proto = rule->proto;
>  
>   if (rule->action == FR_ACT_GOTO &&
>   rcu_access_pointer(rule->ctarget) == NULL)
> -- 
> 2.14.3
>

Re: [PATCH net-next v2 1/1] net: Allow a rule to track originating protocol

2018-02-21 Thread David Miller 
From: Donald Sharp 
Date: Tue, 20 Feb 2018 08:55:58 -0500

> Allow a rule that is being added/deleted/modified or
> dumped to contain the originating protocol's id.
> 
> The protocol is handled just like a routes originating
> protocol is.  This is especially useful because there
> is starting to be a plethora of different user space
> programs adding rules.
> 
> Allow the vrf device to specify that the kernel is the originator
> of the rule created for this device.
> 
> Signed-off-by: Donald Sharp 

Looks good, applied, thanks Donald.

[PATCH net-next v2 1/1] net: Allow a rule to track originating protocol

2018-02-20 Thread Donald Sharp 
Allow a rule that is being added/deleted/modified or
dumped to contain the originating protocol's id.

The protocol is handled just like a routes originating
protocol is.  This is especially useful because there
is starting to be a plethora of different user space
programs adding rules.

Allow the vrf device to specify that the kernel is the originator
of the rule created for this device.

Signed-off-by: Donald Sharp 
---
 drivers/net/vrf.c  | 1 +
 include/net/fib_rules.h| 3 ++-
 include/uapi/linux/fib_rules.h | 2 +-
 net/core/fib_rules.c   | 7 ++-
 4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
index 139c61c8244a..ec6d2d623b60 100644
--- a/drivers/net/vrf.c
+++ b/drivers/net/vrf.c
@@ -1175,6 +1175,7 @@ static int vrf_fib_rule(const struct net_device *dev, 
__u8 family, bool add_it)
memset(frh, 0, sizeof(*frh));
frh->family = family;
frh->action = FR_ACT_TO_TBL;
+   frh->proto = RTPROT_KERNEL;
 
if (nla_put_u8(skb, FRA_L3MDEV, 1))
goto nla_put_failure;
diff --git a/include/net/fib_rules.h b/include/net/fib_rules.h
index 648caf90ec07..b166ef07e6d4 100644
--- a/include/net/fib_rules.h
+++ b/include/net/fib_rules.h
@@ -26,7 +26,8 @@ struct fib_rule {
u32 table;
u8  action;
u8  l3mdev;
-   /* 2 bytes hole, try to use */
+   u8  proto;
+   /* 1 byte hole, try to use */
u32 target;
__be64  tun_id;
struct fib_rule __rcu   *ctarget;
diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
index 2b642bf9b5a0..925539172d5b 100644
--- a/include/uapi/linux/fib_rules.h
+++ b/include/uapi/linux/fib_rules.h
@@ -23,8 +23,8 @@ struct fib_rule_hdr {
__u8tos;
 
__u8table;
+   __u8proto;
__u8res1;   /* reserved */
-   __u8res2;   /* reserved */
__u8action;
 
__u32   flags;
diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 98e1066c3d55..c1d4ab5b2d9f 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -51,6 +51,7 @@ int fib_default_rule_add(struct fib_rules_ops *ops,
r->pref = pref;
r->table = table;
r->flags = flags;
+   r->proto = RTPROT_KERNEL;
r->fr_net = ops->fro_net;
r->uid_range = fib_kuid_range_unset;
 
@@ -465,6 +466,7 @@ int fib_nl_newrule(struct sk_buff *skb, struct nlmsghdr 
*nlh,
}
refcount_set(>refcnt, 1);
rule->fr_net = net;
+   rule->proto = frh->proto;
 
rule->pref = tb[FRA_PRIORITY] ? nla_get_u32(tb[FRA_PRIORITY])
  : fib_default_rule_pref(ops);
@@ -664,6 +666,9 @@ int fib_nl_delrule(struct sk_buff *skb, struct nlmsghdr 
*nlh,
}
 
list_for_each_entry(rule, >rules_list, list) {
+   if (frh->proto && (frh->proto != rule->proto))
+   continue;
+
if (frh->action && (frh->action != rule->action))
continue;
 
@@ -808,9 +813,9 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct 
fib_rule *rule,
if (nla_put_u32(skb, FRA_SUPPRESS_PREFIXLEN, rule->suppress_prefixlen))
goto nla_put_failure;
frh->res1 = 0;
-   frh->res2 = 0;
frh->action = rule->action;
frh->flags = rule->flags;
+   frh->proto = rule->proto;
 
if (rule->action == FR_ACT_GOTO &&
rcu_access_pointer(rule->ctarget) == NULL)
-- 
2.14.3