Message ID | CAP8BHkG6=KuU9K7=sGsZftMDe58eTYByRu2DZ+j+KNqcfsOfAA@mail.gmail.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, Nov 12, 2013 at 04:39:10PM +0530, Kamala R wrote: > Hi, > > Sure, here it is. > > --- linux-3.12/net/ipv6/route.c.orig 2013-11-12 16:23:46.000000000 +0530 > +++ linux-3.12/net/ipv6/route.c 2013-11-12 16:30:51.000000000 +0530 > @@ -1570,9 +1570,13 @@ int ip6_route_add(struct fib6_config *cf > switch (cfg->fc_type) { > case RTN_BLACKHOLE: > rt->dst.error = -EINVAL; > + rt->dst.input = dst_discard; > + rt->dst.discard = dst_discard; > break; > case RTN_PROHIBIT: > rt->dst.error = -EACCES; > + rt->dst.input = ip6_pkt_prohibit; > + rt->dst.output = ip6_pkt_prohibit_out; > break; > case RTN_THROW: > rt->dst.error = -EAGAIN; > > Is this ok ? I woud move all the initialization of the function pointer into the switch-case. You could merge the case RTN_THROW with the default one by just using a ternary statement to initialize dst.error. Your patch must be well-formed to get included into the kernel. For that you should base your patch ontop net-next or net, write a proper commit message and send the git format-patch generated patch to this list. Here some hints: <https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/Documentation/SubmittingPatches> You can check if your formatting is correct by using scripts/checkpatch --strict. Let me know if there are any problems with that. Thank you, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, I have one quick question before I send the patch across. I noticed that "ip -6 route show" shows an error -22 in the output which signifies -EINVAL associated with blackhole routes. This behaviour is not consistent with that of "ip route show" that shows no such error for a blackhole route. Does this qualify a bug that needs fixing ? Regards, Kamala On Tue, Nov 12, 2013 at 7:24 PM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > On Tue, Nov 12, 2013 at 04:39:10PM +0530, Kamala R wrote: >> Hi, >> >> Sure, here it is. >> >> --- linux-3.12/net/ipv6/route.c.orig 2013-11-12 16:23:46.000000000 +0530 >> +++ linux-3.12/net/ipv6/route.c 2013-11-12 16:30:51.000000000 +0530 >> @@ -1570,9 +1570,13 @@ int ip6_route_add(struct fib6_config *cf >> switch (cfg->fc_type) { >> case RTN_BLACKHOLE: >> rt->dst.error = -EINVAL; >> + rt->dst.input = dst_discard; >> + rt->dst.discard = dst_discard; >> break; >> case RTN_PROHIBIT: >> rt->dst.error = -EACCES; >> + rt->dst.input = ip6_pkt_prohibit; >> + rt->dst.output = ip6_pkt_prohibit_out; >> break; >> case RTN_THROW: >> rt->dst.error = -EAGAIN; >> >> Is this ok ? > > I woud move all the initialization of the function pointer into the > switch-case. You could merge the case RTN_THROW with the default one by just > using a ternary statement to initialize dst.error. > > Your patch must be well-formed to get included into the > kernel. For that you should base your patch ontop net-next > or net, write a proper commit message and send the git > format-patch generated patch to this list. Here some hints: > <https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/Documentation/SubmittingPatches> > > You can check if your formatting is correct by using scripts/checkpatch > --strict. > > Let me know if there are any problems with that. > > Thank you, > > Hannes > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi! On Thu, Nov 14, 2013 at 01:50:20PM +0530, Kamala R wrote: > I have one quick question before I send the patch across. I noticed > that "ip -6 route show" shows an error -22 in the output which > signifies -EINVAL associated with blackhole routes. This behaviour is > not consistent with that of "ip route show" that shows no such error > for a blackhole route. Does this qualify a bug that needs fixing ? You mean that we don't show the error for IPv4 routes? Is this a kernel or iproute problem? Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, That's right. We don't show the error for IPv4 routes as it follows a different path in kernel while dumping the information as compared with IPv6. Therefore, "ip route show" does not show an error while "ip -6 route show" does. So it looks to me that this a kernel problem which needs to be fixed for consistent behavior. The simplest way to fix this seems to be to set the error to zero while dumping the information in the v6 path. I have tested this solution and found that it works fine. Do you think this is the way to go ? Regards, Kamala On Thu, Nov 14, 2013 at 7:41 PM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > Hi! > > On Thu, Nov 14, 2013 at 01:50:20PM +0530, Kamala R wrote: >> I have one quick question before I send the patch across. I noticed >> that "ip -6 route show" shows an error -22 in the output which >> signifies -EINVAL associated with blackhole routes. This behaviour is >> not consistent with that of "ip route show" that shows no such error >> for a blackhole route. Does this qualify a bug that needs fixing ? > > You mean that we don't show the error for IPv4 routes? Is this a kernel or > iproute problem? > > Greetings, > > Hannes > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi! On Fri, Nov 15, 2013 at 03:06:28PM +0530, Kamala R wrote: > That's right. We don't show the error for IPv4 routes as it follows a > different path in kernel while dumping the information as compared > with IPv6. Therefore, "ip route show" does not show an error while "ip > -6 route show" does. So it looks to me that this a kernel problem > which needs to be fixed for consistent behavior. The simplest way to > fix this seems to be to set the error to zero while dumping the > information in the v6 path. I have tested this solution and found that > it works fine. Do you think this is the way to go ? If I understand you correctly you propose to drop the output of the error attribute for IPv6 routes too? It is not that important that those two outputs are identical and if you make a change, please introduce the error propagation for IPv4 so one can see the socket errors for those routes, too. I wouldn't drop those for IPv6 just for consistency reasons. Greetings, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, I have sent across the patch on a separate thread. The subject of the mail is : [PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes Could you review ? Thanks, Kamala On Fri, Nov 15, 2013 at 7:18 PM, Hannes Frederic Sowa <hannes@stressinduktion.org> wrote: > Hi! > > On Fri, Nov 15, 2013 at 03:06:28PM +0530, Kamala R wrote: >> That's right. We don't show the error for IPv4 routes as it follows a >> different path in kernel while dumping the information as compared >> with IPv6. Therefore, "ip route show" does not show an error while "ip >> -6 route show" does. So it looks to me that this a kernel problem >> which needs to be fixed for consistent behavior. The simplest way to >> fix this seems to be to set the error to zero while dumping the >> information in the v6 path. I have tested this solution and found that >> it works fine. Do you think this is the way to go ? > > If I understand you correctly you propose to drop the output of the error > attribute for IPv6 routes too? It is not that important that those two > outputs are identical and if you make a change, please introduce the > error propagation for IPv4 so one can see the socket errors for those > routes, too. I wouldn't drop those for IPv6 just for consistency reasons. > > Greetings, > > Hannes > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Kamala R <kamala@aristanetworks.com> Date: Wed, 20 Nov 2013 10:18:04 +0530 > I have sent across the patch on a separate thread. The subject of the mail is : > > [PATCH 1/1] IPv6: Fixed support for blackhole and prohibit routes > > Could you review ? Can you just be patient? You only posted the patch in the last day, many people review patches in their spare time. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- linux-3.12/net/ipv6/route.c.orig 2013-11-12 16:23:46.000000000 +0530 +++ linux-3.12/net/ipv6/route.c 2013-11-12 16:30:51.000000000 +0530 @@ -1570,9 +1570,13 @@ int ip6_route_add(struct fib6_config *cf switch (cfg->fc_type) { case RTN_BLACKHOLE: rt->dst.error = -EINVAL; + rt->dst.input = dst_discard; + rt->dst.discard = dst_discard; break; case RTN_PROHIBIT: rt->dst.error = -EACCES; + rt->dst.input = ip6_pkt_prohibit; + rt->dst.output = ip6_pkt_prohibit_out; break; case RTN_THROW: rt->dst.error = -EAGAIN;