Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[David Ahern]

On 3/7/18 9:51 AM, Stephen Hemminger wrote:
> On Tue,  6 Mar 2018 17:03:54 -0800
> Stephen Hemminger  wrote:
> 
>> From: Stephen Hemminger 
>>
>> Every non-multicast route prints an error message.
>> Kernel doesn't filter out unicast routes, it is up to filter function
>> to do this.
>>
>> Signed-off-by: Stephen Hemminger 
> 
> I found the issue (in kernel) but not sure how to deal with it.
> If kernel is built without multicast routing configured !CONFIG_IP_MROUTE
> then the netlink request to return multicast routes will return all routes!
> 
> This is because in the kernel the way route dump works is that each
> address family registers a callback to dump routes for a specific address
> family.  If that address family is not registered then the fall back
> is to address family PF_UNSPEC which has a handler that dumps all
> routes.

If I ask the kernel for IP{6}MR address family and the kernel returns a
dump of all address families, that seems like a kernel bug to me. From a
short review it seems to be that way for a long time. I guess iproute2
has no choice but to adapt to the kernel design.

Are you going to resubmit this series?


> 
> Unfortunately, changing that behavior in kernel will certainly break
> some user. And there is no direct way to determine multicast routing
> is enabled in ip mroute code.
> 
> Maybe just change the message in ip mroute to do:
> 
> diff --git a/ip/ipmroute.c b/ip/ipmroute.c
> index aa5029b44f41..31b9bfe95596 100644
> --- a/ip/ipmroute.c
> +++ b/ip/ipmroute.c
> @@ -76,9 +76,8 @@ int print_mroute(const struct sockaddr_nl *who, struct 
> nlmsghdr *n, void *arg)
> return -1;
> }
> if (r->rtm_type != RTN_MULTICAST) {
> -   fprintf(stderr, "Not a multicast route (type: %s)\n",
> -   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
> -   return 0;
> +   fprintf(stderr, "Multicast routing does not appear to be 
> enabled\n");
> +   return -1;
> }
>  
> parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
> 

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Stephen Hemminger]

On Tue,  6 Mar 2018 17:03:54 -0800
Stephen Hemminger  wrote:

> From: Stephen Hemminger 
> 
> Every non-multicast route prints an error message.
> Kernel doesn't filter out unicast routes, it is up to filter function
> to do this.
> 
> Signed-off-by: Stephen Hemminger 

I found the issue (in kernel) but not sure how to deal with it.
If kernel is built without multicast routing configured !CONFIG_IP_MROUTE
then the netlink request to return multicast routes will return all routes!

This is because in the kernel the way route dump works is that each
address family registers a callback to dump routes for a specific address
family.  If that address family is not registered then the fall back
is to address family PF_UNSPEC which has a handler that dumps all
routes.

Unfortunately, changing that behavior in kernel will certainly break
some user. And there is no direct way to determine multicast routing
is enabled in ip mroute code.

Maybe just change the message in ip mroute to do:

diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index aa5029b44f41..31b9bfe95596 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -76,9 +76,8 @@ int print_mroute(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
return -1;
}
if (r->rtm_type != RTN_MULTICAST) {
-   fprintf(stderr, "Not a multicast route (type: %s)\n",
-   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
-   return 0;
+   fprintf(stderr, "Multicast routing does not appear to be 
enabled\n");
+   return -1;
}
 
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[David Ahern]

On 3/7/18 9:03 AM, Stephen Hemminger wrote:
> On Tue,  6 Mar 2018 17:03:54 -0800
> Stephen Hemminger  wrote:
> 
>> From: Stephen Hemminger 
>>
>> Every non-multicast route prints an error message.
>> Kernel doesn't filter out unicast routes, it is up to filter function
>> to do this.
>>
>> Signed-off-by: Stephen Hemminger 
>> ---
>>  ip/ipmroute.c | 7 +++
>>  1 file changed, 3 insertions(+), 4 deletions(-)
>>
>> diff --git a/ip/ipmroute.c b/ip/ipmroute.c
>> index aa5029b44f41..03ca0575e571 100644
>> --- a/ip/ipmroute.c
>> +++ b/ip/ipmroute.c
>> @@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
>> nlmsghdr *n, void *arg)
>>  fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
>>  return -1;
>>  }
>> -if (r->rtm_type != RTN_MULTICAST) {
>> -fprintf(stderr, "Not a multicast route (type: %s)\n",
>> -rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
>> +
>> +if (r->rtm_type != RTN_MULTICAST)
>>  return 0;
>> -}
>>  
>>  parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
>>  table = rtm_get_table(r, tb);
>>  
>> +
>>  if (filter.tb > 0 && filter.tb != table)
>>  return 0;
>>  
> 
> Actually, this is a recent kernel regression. Should be fixed there
> 

from Yuval's recent multicast convergence changes?

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Stephen Hemminger]

On Tue,  6 Mar 2018 17:03:54 -0800
Stephen Hemminger  wrote:

> From: Stephen Hemminger 
> 
> Every non-multicast route prints an error message.
> Kernel doesn't filter out unicast routes, it is up to filter function
> to do this.
> 
> Signed-off-by: Stephen Hemminger 
> ---
>  ip/ipmroute.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/ip/ipmroute.c b/ip/ipmroute.c
> index aa5029b44f41..03ca0575e571 100644
> --- a/ip/ipmroute.c
> +++ b/ip/ipmroute.c
> @@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
> nlmsghdr *n, void *arg)
>   fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
>   return -1;
>   }
> - if (r->rtm_type != RTN_MULTICAST) {
> - fprintf(stderr, "Not a multicast route (type: %s)\n",
> - rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
> +
> + if (r->rtm_type != RTN_MULTICAST)
>   return 0;
> - }
>  
>   parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
>   table = rtm_get_table(r, tb);
>  
> +
>   if (filter.tb > 0 && filter.tb != table)
>   return 0;
>  

Actually, this is a recent kernel regression. Should be fixed there

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Sergei Shtylyov]

On 03/07/2018 06:54 PM, Stephen Hemminger wrote:

>> Hello!
>>
>> On 3/7/2018 4:03 AM, Stephen Hemminger wrote:
>>
>>> From: Stephen Hemminger 
>>>
>>> Every non-multicast route prints an error message.
>>> Kernel doesn't filter out unicast routes, it is up to filter function
>>> to do this.
>>>
>>> Signed-off-by: Stephen Hemminger 
>>> ---
>>>   ip/ipmroute.c | 7 +++
>>>   1 file changed, 3 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/ip/ipmroute.c b/ip/ipmroute.c
>>> index aa5029b44f41..03ca0575e571 100644
>>> --- a/ip/ipmroute.c
>>> +++ b/ip/ipmroute.c
>>> @@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
>>> nlmsghdr *n, void *arg)
>>> fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
>>> return -1;
>>> }
>>> -   if (r->rtm_type != RTN_MULTICAST) {
>>> -   fprintf(stderr, "Not a multicast route (type: %s)\n",
>>> -   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
>>> +
>>> +   if (r->rtm_type != RTN_MULTICAST)
>>> return 0;
>>> -   }
>>>   
>>> parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
>>> table = rtm_get_table(r, tb);
>>>   
>>> +  
>>
>> Why?
>>
>>> if (filter.tb > 0 && filter.tb != table)
>>> return 0;
>>> 
>>
>> MBR, Sergei
> 
> The kernel dumps all routes in response to the RTM_GETROUTE and therefore all 
> routes show
> up in print_mroute.  On my system (which has no mcast), I get this with 
> standard 4.14
> kernel and ip commands.

   I was merely asking about a double new line... :-)

MBR, Sergei

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Stephen Hemminger]

On Wed, 7 Mar 2018 11:43:33 +0300
Sergei Shtylyov  wrote:

> Hello!
> 
> On 3/7/2018 4:03 AM, Stephen Hemminger wrote:
> 
> > From: Stephen Hemminger 
> > 
> > Every non-multicast route prints an error message.
> > Kernel doesn't filter out unicast routes, it is up to filter function
> > to do this.
> > 
> > Signed-off-by: Stephen Hemminger 
> > ---
> >   ip/ipmroute.c | 7 +++
> >   1 file changed, 3 insertions(+), 4 deletions(-)
> > 
> > diff --git a/ip/ipmroute.c b/ip/ipmroute.c
> > index aa5029b44f41..03ca0575e571 100644
> > --- a/ip/ipmroute.c
> > +++ b/ip/ipmroute.c
> > @@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
> > nlmsghdr *n, void *arg)
> > fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
> > return -1;
> > }
> > -   if (r->rtm_type != RTN_MULTICAST) {
> > -   fprintf(stderr, "Not a multicast route (type: %s)\n",
> > -   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
> > +
> > +   if (r->rtm_type != RTN_MULTICAST)
> > return 0;
> > -   }
> >   
> > parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
> > table = rtm_get_table(r, tb);
> >   
> > +  
> 
> Why?
> 
> > if (filter.tb > 0 && filter.tb != table)
> > return 0;
> > 
> 
> MBR, Sergei

The kernel dumps all routes in response to the RTM_GETROUTE and therefore all 
routes show
up in print_mroute.  On my system (which has no mcast), I get this with 
standard 4.14
kernel and ip commands.

$ ip mroute
Not a multicast route (type: unicast)
Not a multicast route (type: unicast)
Not a multicast route (type: unicast)
Not a multicast route (type: unicast)
Not a multicast route (type: unicast)
Not a multicast route (type: broadcast)
Not a multicast route (type: local)
Not a multicast route (type: local)
Not a multicast route (type: broadcast)
Not a multicast route (type: broadcast)
Not a multicast route (type: local)

Re: [PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Sergei Shtylyov]

Hello!

On 3/7/2018 4:03 AM, Stephen Hemminger wrote:


From: Stephen Hemminger 

Every non-multicast route prints an error message.
Kernel doesn't filter out unicast routes, it is up to filter function
to do this.

Signed-off-by: Stephen Hemminger 
---
  ip/ipmroute.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index aa5029b44f41..03ca0575e571 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
return -1;
}
-   if (r->rtm_type != RTN_MULTICAST) {
-   fprintf(stderr, "Not a multicast route (type: %s)\n",
-   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
+
+   if (r->rtm_type != RTN_MULTICAST)
return 0;
-   }
  
  	parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);

table = rtm_get_table(r, tb);
  
+


   Why?


if (filter.tb > 0 && filter.tb != table)
return 0;
  


MBR, Sergei

[PATCH iproute2-next 2/3] ipmroute: don't complain about unicast routes

[Stephen Hemminger]

From: Stephen Hemminger 

Every non-multicast route prints an error message.
Kernel doesn't filter out unicast routes, it is up to filter function
to do this.

Signed-off-by: Stephen Hemminger 
---
 ip/ipmroute.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ip/ipmroute.c b/ip/ipmroute.c
index aa5029b44f41..03ca0575e571 100644
--- a/ip/ipmroute.c
+++ b/ip/ipmroute.c
@@ -75,15 +75,14 @@ int print_mroute(const struct sockaddr_nl *who, struct 
nlmsghdr *n, void *arg)
fprintf(stderr, "BUG: wrong nlmsg len %d\n", len);
return -1;
}
-   if (r->rtm_type != RTN_MULTICAST) {
-   fprintf(stderr, "Not a multicast route (type: %s)\n",
-   rtnl_rtntype_n2a(r->rtm_type, b1, sizeof(b1)));
+
+   if (r->rtm_type != RTN_MULTICAST)
return 0;
-   }
 
parse_rtattr(tb, RTA_MAX, RTM_RTA(r), len);
table = rtm_get_table(r, tb);
 
+
if (filter.tb > 0 && filter.tb != table)
return 0;
 
-- 
2.16.1