> On 15 January 2018 at 21:18, Lorenzo Bianconi > <lorenzo.bianc...@redhat.com> wrote: >>> On Sun, Jan 14, 2018 at 03:50:54PM +0100, Lorenzo Bianconi wrote: >>>> Although this issue is harmless since that code path is protected by the >>>> check on l2tp_nl_cmd_ops[]/l2tp_nl_cmd_ops[]->session_create(), fix error >>>> handling for L2TP_PWTYPE_IP/default case in l2tp_nl_cmd_session_create() >>>> >>>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianc...@redhat.com> >>>> --- >>>> net/l2tp/l2tp_netlink.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c >>>> index e1ca29f79821..48b5bf30ec50 100644 >>>> --- a/net/l2tp/l2tp_netlink.c >>>> +++ b/net/l2tp/l2tp_netlink.c >>>> @@ -635,7 +635,7 @@ static int l2tp_nl_cmd_session_create(struct sk_buff >>>> *skb, struct genl_info *inf >>>> case L2TP_PWTYPE_IP: >>>> default: >>>> ret = -EPROTONOSUPPORT; >>>> - break; >>>> + goto out_tunnel; >>>> } >>>> >>> Not sure if this change is really worthwhile. As you noted, this is >>> unreachable code. And this switch should better be removed entirely: >>> it doesn't do anything for supported pseudo-wires. >>> >>> And if PWTYPE_ETH_VLAN were to be implemented, it should perform its >>> configuration consistency checking in its own PW specific code, not in >>> the genl handler. >>> >> >> Personally I would prefer to not remove some code that could be useful >> for a future implementation, but just fix it if it presents issues to >> address. >> Anyway we can simply drop this patch from the series and I can send a >> new one to remove the switch entirely. >> >> @James what do you think? > > Keep the patch series focused. If you read the patch series as a set, > this patch stands out as not fitting the purpose of the series. I > agree with Guillaume. >
Ok, fine. What about the fix? Do you prefer to remove the switch or just fix it? >> >> Regards, >> Lorenzo >> >>> Anyway, removing this switch isn't the purpose of this series, so I >>> think you can drop this patch. > > I agree.
