| Message ID | 87egg8ryia.fsf@x220.int.ebiederm.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
Hello. On 11/02/2015 10:29 PM, Eric W. Biederman wrote: > Currently the multipath code has a nasty failure mode in that it will > fail to notice link down or administrative device down and will > instead black hole packets instead of sending them to their nexthop > destination. > > Half the point of multipath is to gracefully handle forwarding path > failures and as the current code does not handle forwarding failures the > current code is dangerous to use. > > As mpls multipath has never been exported to userspace and as the > implementation was not complete before the merge window disable the mpls > multipath code by rejecting all multipath configuration requests. This > will give us another kernel development cycle to cleanly sort out the > issues, without any bad precedents to worry about. > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > net/mpls/af_mpls.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c > index c70d750148b6..893cd2dc3979 100644 > --- a/net/mpls/af_mpls.c > +++ b/net/mpls/af_mpls.c > @@ -1162,6 +1162,8 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, > { > cfg->rc_mp = nla_data(nla); > cfg->rc_mp_len = nla_len(nla); > + /* Fail until multipath support is complete */ > + goto errout; > break; Forgot to delete *break*? > } > default: > MBR, Sergei -- 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
Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes: > Hello. >> >> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c >> index c70d750148b6..893cd2dc3979 100644 >> --- a/net/mpls/af_mpls.c >> +++ b/net/mpls/af_mpls.c >> @@ -1162,6 +1162,8 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, >> { >> cfg->rc_mp = nla_data(nla); >> cfg->rc_mp_len = nla_len(nla); >> + /* Fail until multipath support is complete */ >> + goto errout; >> break; > > Forgot to delete *break*? Nope. I did that deliberately, because this code is not supposed to stay this way for long. Eric -- 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
On 11/2/15, 10:09 PM, Eric W. Biederman wrote: > Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes: > >> Hello. >>> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c >>> index c70d750148b6..893cd2dc3979 100644 >>> --- a/net/mpls/af_mpls.c >>> +++ b/net/mpls/af_mpls.c >>> @@ -1162,6 +1162,8 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, >>> { >>> cfg->rc_mp = nla_data(nla); >>> cfg->rc_mp_len = nla_len(nla); >>> + /* Fail until multipath support is complete */ >>> + goto errout; >>> break; >> Forgot to delete *break*? > Nope. I did that deliberately, because this code is not supposed to > stay this way for long. Eric, this is only until dead route support is added correct ?. If yes, then my v2 on dead route/nh support is out. I know david is only taking bug fixes right now... and I thought the dead route/nh is a bug fix. If you are ok with v2, we could skip this patch ?. If david decides to not pick it up in net-next, i was hoping to submit it as a bug-fix to net. -- 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
On 03/11/15 14:03, roopa wrote: > On 11/2/15, 10:09 PM, Eric W. Biederman wrote: >> Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> writes: >> >>> Hello. >>>> diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c >>>> index c70d750148b6..893cd2dc3979 100644 >>>> --- a/net/mpls/af_mpls.c >>>> +++ b/net/mpls/af_mpls.c >>>> @@ -1162,6 +1162,8 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, >>>> { >>>> cfg->rc_mp = nla_data(nla); >>>> cfg->rc_mp_len = nla_len(nla); >>>> + /* Fail until multipath support is complete */ >>>> + goto errout; >>>> break; >>> Forgot to delete *break*? >> Nope. I did that deliberately, because this code is not supposed to >> stay this way for long. > Eric, this is only until dead route support is added correct ?. > If yes, then my v2 on dead route/nh support is out. I know david is only taking bug fixes right now... > and I thought the dead route/nh is a bug fix. If you are ok with v2, we could skip this patch ?. Roopa, I think we should take our time with the dead nexthop changes, and Eric's patch gives us that time. Thanks, Rob > > If david decides to not pick it up in net-next, i was hoping to submit it as a bug-fix to net. > -- 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
diff --git a/net/mpls/af_mpls.c b/net/mpls/af_mpls.c index c70d750148b6..893cd2dc3979 100644 --- a/net/mpls/af_mpls.c +++ b/net/mpls/af_mpls.c @@ -1162,6 +1162,8 @@ static int rtm_to_route_config(struct sk_buff *skb, struct nlmsghdr *nlh, { cfg->rc_mp = nla_data(nla); cfg->rc_mp_len = nla_len(nla); + /* Fail until multipath support is complete */ + goto errout; break; } default:
Currently the multipath code has a nasty failure mode in that it will fail to notice link down or administrative device down and will instead black hole packets instead of sending them to their nexthop destination. Half the point of multipath is to gracefully handle forwarding path failures and as the current code does not handle forwarding failures the current code is dangerous to use. As mpls multipath has never been exported to userspace and as the implementation was not complete before the merge window disable the mpls multipath code by rejecting all multipath configuration requests. This will give us another kernel development cycle to cleanly sort out the issues, without any bad precedents to worry about. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- net/mpls/af_mpls.c | 2 ++ 1 file changed, 2 insertions(+)