> 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? Regards, Lorenzo > Anyway, removing this switch isn't the purpose of this series, so I > think you can drop this patch.