On Mon, Jan 15, 2018 at 07:43:18PM +0100, Lorenzo Bianconi wrote: > > On Sun, Jan 14, 2018 at 03:50:56PM +0100, Lorenzo Bianconi wrote: > >> --- a/net/l2tp/l2tp_core.h > >> +++ b/net/l2tp/l2tp_core.h > >> @@ -302,6 +302,17 @@ static inline void l2tp_session_dec_refcount(struct > >> l2tp_session *session) > >> l2tp_session_free(session); > >> } > >> > >> +static inline int l2tp_get_l2specific_len(struct l2tp_session *session) > >> +{ > >> + switch (session->l2specific_type) { > >> + case L2TP_L2SPECTYPE_NONE: > >> + return 0; > >> + case L2TP_L2SPECTYPE_DEFAULT: > >> + default: > >> + return 4; > >> + } > >> +} > >> > > The data path only compares ->l2specific_type to L2SPECTYPE_DEFAULT and > > treats any other value as L2SPECTYPE_NONE. Therefore, we should keep > > this logic here and return 0 for unknown types. > > The data path only compares l2specific_type to L2SPECTYPE_DEFAULT > since in the other supported case (L2SPECTYPE_NONE) there is no action > required. Moreover L2SPECTYPE_DEFAULT is default configured value if > the user does not provide any value for l2specific_type so there are > no 'unknown' types and I thought L2TP_L2SPECTYPE_DEFAULT was a better > choice for default value > Yes, but what I meant is that the data patch treats unknow values as L2SPECTYPE_NONE, while l2tp_get_l2specific_len() now treats them as L2TP_L2SPECTYPE_DEFAULT. I'd just prefer to avoid that inconsistency; it makes it easier to reason about the code.
But if you really prefer L2SPECTYPE_DEFAULT, then fine. Unless someone messes with new l2spec types, we should never reach this case.