-----Original Message----- From: Guillaume Nault gnault@redhat.com Sent: środa, 29 marca 2023 17:26 To: Drewek, Wojciech wojciech.drewek@intel.com Cc: Andrea Righi andrea.righi@canonical.com; David S. Miller davem@davemloft.net; Eric Dumazet edumazet@google.com; Jakub Kicinski kuba@kernel.org; Paolo Abeni pabeni@redhat.com; Shuah Khan shuah@kernel.org; netdev@vger.kernel.org; linux-kselftest@vger.kernel.org; linux-kernel@vger.kernel.org Subject: Re: selftests: net: l2tp.sh regression starting with 6.1-rc1
On Wed, Mar 29, 2023 at 02:16:37PM +0000, Drewek, Wojciech wrote:
Hi,
Modifying UAPI was not a good idea although the patch should not break userspace (related discussion [1]). We could revert this patch with one additional change (include l2tp.h in net/sched/cls_flower.c) but then again, modifying UAPI. This patch was mostly cosmetic anyway. Second option is to try to fix the automatic load. I'm not an expert but I think MODULE_ALIAS_NET_PF_PROTO macro is somehow responsible for that. I noticed some comments saying that "__stringify doesn't like enums" (this macro is using _stringify) and my patch defined IPPROTO_L2TP in enum. We can just replace IPPROTO_L2TP with 115 (where this macro is used) in order to fix this. I'm going to give it a try and will let you know.
Yes, the modules aliases now have symbolic names:
$ modinfo l2tp_ip l2tp_ip6 | grep alias alias: net-pf-2-proto-IPPROTO_L2TP alias: net-pf-2-proto-2-type-IPPROTO_L2TP alias: net-pf-10-proto-IPPROTO_L2TP alias: net-pf-10-proto-2-type-IPPROTO_L2TP
Therefore, 'request_module("net-pf-%d-proto-%d-type-%d")' can't find them.
My personal preference is for the second option: fix module loading by using plain numbers in MODULE_ALIAS_*. We can always keep the symbolic names in comments.
---- >8 ----
diff --git a/net/l2tp/l2tp_ip.c b/net/l2tp/l2tp_ip.c index 4db5a554bdbd..afe94a390ef0 100644 --- a/net/l2tp/l2tp_ip.c +++ b/net/l2tp/l2tp_ip.c @@ -680,5 +680,5 @@ MODULE_VERSION("1.0"); /* Use the value of SOCK_DGRAM (2) directory, because __stringify doesn't like
- enums
*/ -MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET, 2, IPPROTO_L2TP); -MODULE_ALIAS_NET_PF_PROTO(PF_INET, IPPROTO_L2TP); +MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET, 2, 115 /* IPPROTO_L2TP */); +MODULE_ALIAS_NET_PF_PROTO(PF_INET, 115 /* IPPROTO_L2TP */); diff --git a/net/l2tp/l2tp_ip6.c b/net/l2tp/l2tp_ip6.c index 2478aa60145f..65d106b41951 100644 --- a/net/l2tp/l2tp_ip6.c +++ b/net/l2tp/l2tp_ip6.c @@ -809,5 +809,5 @@ MODULE_VERSION("1.0"); /* Use the value of SOCK_DGRAM (2) directory, because __stringify doesn't like
- enums
*/ -MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET6, 2, IPPROTO_L2TP); -MODULE_ALIAS_NET_PF_PROTO(PF_INET6, IPPROTO_L2TP); +MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET6, 2, 115 /* IPPROTO_L2TP */); +MODULE_ALIAS_NET_PF_PROTO(PF_INET6, 115 /* IPPROTO_L2TP */);
Btw, am I blind or the alias with type was wrong the whole time? pf goes first, then proto and type at the end according to the definition of MODULE_ALIAS_NET_PF_PROTO_TYPE and here type (2) is 2nd and proto (115) is 3rd