| Message ID | 20240229005920.2407409-13-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c05bf0e933129dbb71d057949d90531b03462538 |
| Headers | show |
| Series | selftests: kselftest_harness: support using xfail | expand |
On Wed, Feb 28, 2024 at 04:59 PM -08, Jakub Kicinski wrote: > SCTP does not support IP_LOCAL_PORT_RANGE and we know it, > so use XFAIL instead of SKIP. > > Reviewed-by: Kees Cook <keescook@chromium.org> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > --- > tools/testing/selftests/net/ip_local_port_range.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c > index 6ebd58869a63..193b82745fd8 100644 > --- a/tools/testing/selftests/net/ip_local_port_range.c > +++ b/tools/testing/selftests/net/ip_local_port_range.c > @@ -365,9 +365,6 @@ TEST_F(ip_local_port_range, late_bind) > __u32 range; > __u16 port; > > - if (variant->so_protocol == IPPROTO_SCTP) > - SKIP(return, "SCTP doesn't support IP_BIND_ADDRESS_NO_PORT"); > - > fd = socket(variant->so_domain, variant->so_type, 0); > ASSERT_GE(fd, 0) TH_LOG("socket failed"); > > @@ -414,6 +411,9 @@ TEST_F(ip_local_port_range, late_bind) > ASSERT_TRUE(!err) TH_LOG("close failed"); > } > > +XFAIL_ADD(ip_local_port_range, ip4_stcp, late_bind); > +XFAIL_ADD(ip_local_port_range, ip6_stcp, late_bind); > + > TEST_F(ip_local_port_range, get_port_range) > { > __u16 lo, hi; [wrt our earlier discussion off-list] You were right, this test succeeds if I delete SKIP for SCTP. Turns out IP_LOCAL_PORT_RANGE works for SCTP out of the box after all. What I didn't notice earlier is that sctp_setsockopt() delegates to ip_setsockopt() when level != SOL_SCTP. CC'ing Marcelo & Xin, to confirm that this isn't a problem.
On Thu, Feb 29, 2024 at 3:27 PM Jakub Sitnicki <jakub@cloudflare.com> wrote: > > On Wed, Feb 28, 2024 at 04:59 PM -08, Jakub Kicinski wrote: > > SCTP does not support IP_LOCAL_PORT_RANGE and we know it, > > so use XFAIL instead of SKIP. > > > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> > > --- > > tools/testing/selftests/net/ip_local_port_range.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c > > index 6ebd58869a63..193b82745fd8 100644 > > --- a/tools/testing/selftests/net/ip_local_port_range.c > > +++ b/tools/testing/selftests/net/ip_local_port_range.c > > @@ -365,9 +365,6 @@ TEST_F(ip_local_port_range, late_bind) > > __u32 range; > > __u16 port; > > > > - if (variant->so_protocol == IPPROTO_SCTP) > > - SKIP(return, "SCTP doesn't support IP_BIND_ADDRESS_NO_PORT"); > > - > > fd = socket(variant->so_domain, variant->so_type, 0); > > ASSERT_GE(fd, 0) TH_LOG("socket failed"); > > > > @@ -414,6 +411,9 @@ TEST_F(ip_local_port_range, late_bind) > > ASSERT_TRUE(!err) TH_LOG("close failed"); > > } > > > > +XFAIL_ADD(ip_local_port_range, ip4_stcp, late_bind); > > +XFAIL_ADD(ip_local_port_range, ip6_stcp, late_bind); > > + > > TEST_F(ip_local_port_range, get_port_range) > > { > > __u16 lo, hi; > > [wrt our earlier discussion off-list] > > You were right, this test succeeds if I delete SKIP for SCTP. > Turns out IP_LOCAL_PORT_RANGE works for SCTP out of the box after all. > > What I didn't notice earlier is that sctp_setsockopt() delegates to > ip_setsockopt() when level != SOL_SCTP. > > CC'ing Marcelo & Xin, to confirm that this isn't a problem. Yes, SCTP supports ip_local_port_range by calling inet_sk_get_local_port_range() in sctp_get_port(), similar to TCP/UDP.
On Thu, Feb 29, 2024 at 06:25 PM -05, Xin Long wrote: > On Thu, Feb 29, 2024 at 3:27 PM Jakub Sitnicki <jakub@cloudflare.com> wrote: >> >> On Wed, Feb 28, 2024 at 04:59 PM -08, Jakub Kicinski wrote: >> > SCTP does not support IP_LOCAL_PORT_RANGE and we know it, >> > so use XFAIL instead of SKIP. >> > >> > Reviewed-by: Kees Cook <keescook@chromium.org> >> > Signed-off-by: Jakub Kicinski <kuba@kernel.org> >> > --- >> > tools/testing/selftests/net/ip_local_port_range.c | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c >> > index 6ebd58869a63..193b82745fd8 100644 >> > --- a/tools/testing/selftests/net/ip_local_port_range.c >> > +++ b/tools/testing/selftests/net/ip_local_port_range.c >> > @@ -365,9 +365,6 @@ TEST_F(ip_local_port_range, late_bind) >> > __u32 range; >> > __u16 port; >> > >> > - if (variant->so_protocol == IPPROTO_SCTP) >> > - SKIP(return, "SCTP doesn't support IP_BIND_ADDRESS_NO_PORT"); >> > - >> > fd = socket(variant->so_domain, variant->so_type, 0); >> > ASSERT_GE(fd, 0) TH_LOG("socket failed"); >> > >> > @@ -414,6 +411,9 @@ TEST_F(ip_local_port_range, late_bind) >> > ASSERT_TRUE(!err) TH_LOG("close failed"); >> > } >> > >> > +XFAIL_ADD(ip_local_port_range, ip4_stcp, late_bind); >> > +XFAIL_ADD(ip_local_port_range, ip6_stcp, late_bind); >> > + >> > TEST_F(ip_local_port_range, get_port_range) >> > { >> > __u16 lo, hi; >> >> [wrt our earlier discussion off-list] >> >> You were right, this test succeeds if I delete SKIP for SCTP. >> Turns out IP_LOCAL_PORT_RANGE works for SCTP out of the box after all. >> >> What I didn't notice earlier is that sctp_setsockopt() delegates to >> ip_setsockopt() when level != SOL_SCTP. >> >> CC'ing Marcelo & Xin, to confirm that this isn't a problem. > Yes, SCTP supports ip_local_port_range by calling > inet_sk_get_local_port_range() in sctp_get_port(), similar to TCP/UDP. Well, that's embarassing. I see that I've updated sctp stack to use inet_sk_get_local_port_range() in 91d0b78c5177 ("inet: Add IP_LOCAL_PORT_RANGE socket option"). Thanks for confirming, Xin. It's clearly an overside on my side. That SKIP in tests should have never been there. I will send a fixup.
diff --git a/tools/testing/selftests/net/ip_local_port_range.c b/tools/testing/selftests/net/ip_local_port_range.c index 6ebd58869a63..193b82745fd8 100644 --- a/tools/testing/selftests/net/ip_local_port_range.c +++ b/tools/testing/selftests/net/ip_local_port_range.c @@ -365,9 +365,6 @@ TEST_F(ip_local_port_range, late_bind) __u32 range; __u16 port; - if (variant->so_protocol == IPPROTO_SCTP) - SKIP(return, "SCTP doesn't support IP_BIND_ADDRESS_NO_PORT"); - fd = socket(variant->so_domain, variant->so_type, 0); ASSERT_GE(fd, 0) TH_LOG("socket failed"); @@ -414,6 +411,9 @@ TEST_F(ip_local_port_range, late_bind) ASSERT_TRUE(!err) TH_LOG("close failed"); } +XFAIL_ADD(ip_local_port_range, ip4_stcp, late_bind); +XFAIL_ADD(ip_local_port_range, ip6_stcp, late_bind); + TEST_F(ip_local_port_range, get_port_range) { __u16 lo, hi;