* [PATCH v2 mptcp-next 0/3] mptcp: add more mibs
@ 2023-04-14 14:19 Paolo Abeni
2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Paolo Abeni @ 2023-04-14 14:19 UTC (permalink / raw)
To: mptcp
dumb cover letter for CI's sake.
Slould be applied only after the current tree has been crunched
by syzkaller for a little bit, as this will likely change quite
a bit the timings of some critical path, possibly hiding old races
and make more easy to reach other ones
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/378
v1 -> v2:
- fix mpj self-tests failures
Paolo Abeni (3):
mptcp: introduces more address related mibs
selftests: mptcp: add explicit check for new mibs
selftests: mptcp: centralize stats dumping
net/mptcp/mib.c | 6 +
net/mptcp/mib.h | 18 +++
net/mptcp/options.c | 5 +-
net/mptcp/pm.c | 6 +-
.../testing/selftests/net/mptcp/mptcp_join.sh | 112 +++++++++---------
5 files changed, 91 insertions(+), 56 deletions(-)
--
2.39.2
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs 2023-04-14 14:19 [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Paolo Abeni @ 2023-04-14 14:19 ` Paolo Abeni 2023-04-20 3:57 ` Geliang Tang 2023-04-21 11:32 ` Matthieu Baerts 2023-04-14 14:19 ` [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs Paolo Abeni ` (2 subsequent siblings) 3 siblings, 2 replies; 11+ messages in thread From: Paolo Abeni @ 2023-04-14 14:19 UTC (permalink / raw) To: mptcp Currently we don't track explicitly a few events related to address management suboption handling; this patch adds new mibs for ADD_ADDR and RM_ADDR options tx and for missed tx events due to internal storage exahustion. The self-tests must be updated to properly handle different mibs with the same/shared prefix. Additionally removes a couple of warning tracking the loss event. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/378 Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- net/mptcp/mib.c | 6 ++++++ net/mptcp/mib.h | 18 ++++++++++++++++++ net/mptcp/options.c | 5 ++++- net/mptcp/pm.c | 6 ++++-- .../testing/selftests/net/mptcp/mptcp_join.sh | 4 ++-- 5 files changed, 34 insertions(+), 5 deletions(-) diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c index 0dac2863c6e1..a0990c365a2e 100644 --- a/net/mptcp/mib.c +++ b/net/mptcp/mib.c @@ -34,7 +34,11 @@ static const struct snmp_mib mptcp_snmp_list[] = { SNMP_MIB_ITEM("NoDSSInWindow", MPTCP_MIB_NODSSWINDOW), SNMP_MIB_ITEM("DuplicateData", MPTCP_MIB_DUPDATA), SNMP_MIB_ITEM("AddAddr", MPTCP_MIB_ADDADDR), + SNMP_MIB_ITEM("AddAddrTx", MPTCP_MIB_ADDADDRTX), + SNMP_MIB_ITEM("AddAddrTxDrop", MPTCP_MIB_ADDADDRTXDROP), SNMP_MIB_ITEM("EchoAdd", MPTCP_MIB_ECHOADD), + SNMP_MIB_ITEM("EchoAddTx", MPTCP_MIB_ECHOADDTX), + SNMP_MIB_ITEM("EchoAddTxDrop", MPTCP_MIB_ECHOADDTXDROP), SNMP_MIB_ITEM("PortAdd", MPTCP_MIB_PORTADD), SNMP_MIB_ITEM("AddAddrDrop", MPTCP_MIB_ADDADDRDROP), SNMP_MIB_ITEM("MPJoinPortSynRx", MPTCP_MIB_JOINPORTSYNRX), @@ -44,6 +48,8 @@ static const struct snmp_mib mptcp_snmp_list[] = { SNMP_MIB_ITEM("MismatchPortAckRx", MPTCP_MIB_MISMATCHPORTACKRX), SNMP_MIB_ITEM("RmAddr", MPTCP_MIB_RMADDR), SNMP_MIB_ITEM("RmAddrDrop", MPTCP_MIB_RMADDRDROP), + SNMP_MIB_ITEM("RmAddrTx", MPTCP_MIB_RMADDRTX), + SNMP_MIB_ITEM("RmAddrTxDrop", MPTCP_MIB_RMADDRTXDROP), SNMP_MIB_ITEM("RmSubflow", MPTCP_MIB_RMSUBFLOW), SNMP_MIB_ITEM("MPPrioTx", MPTCP_MIB_MPPRIOTX), SNMP_MIB_ITEM("MPPrioRx", MPTCP_MIB_MPPRIORX), diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h index 2be3596374f4..cae71d947252 100644 --- a/net/mptcp/mib.h +++ b/net/mptcp/mib.h @@ -27,7 +27,15 @@ enum linux_mptcp_mib_field { MPTCP_MIB_NODSSWINDOW, /* Segments not in MPTCP windows */ MPTCP_MIB_DUPDATA, /* Segments discarded due to duplicate DSS */ MPTCP_MIB_ADDADDR, /* Received ADD_ADDR with echo-flag=0 */ + MPTCP_MIB_ADDADDRTX, /* Sent ADD_ADDR with echo-flag=0 */ + MPTCP_MIB_ADDADDRTXDROP, /* ADD_ADDR with echo-flag=0 not send due to + * resource exhaustion + */ MPTCP_MIB_ECHOADD, /* Received ADD_ADDR with echo-flag=1 */ + MPTCP_MIB_ECHOADDTX, /* Send ADD_ADDR with echo-flag=1 */ + MPTCP_MIB_ECHOADDTXDROP, /* ADD_ADDR with echo-flag=1 not send due + * to resource exhaustion + */ MPTCP_MIB_PORTADD, /* Received ADD_ADDR with a port-number */ MPTCP_MIB_ADDADDRDROP, /* Dropped incoming ADD_ADDR */ MPTCP_MIB_JOINPORTSYNRX, /* Received a SYN MP_JOIN with a different port-number */ @@ -37,6 +45,8 @@ enum linux_mptcp_mib_field { MPTCP_MIB_MISMATCHPORTACKRX, /* Received an ACK MP_JOIN with a mismatched port-number */ MPTCP_MIB_RMADDR, /* Received RM_ADDR */ MPTCP_MIB_RMADDRDROP, /* Dropped incoming RM_ADDR */ + MPTCP_MIB_RMADDRTX, /* Sent RM_ADDR */ + MPTCP_MIB_RMADDRTXDROP, /* RM_ADDR not sent due to resource exhaustion */ MPTCP_MIB_RMSUBFLOW, /* Remove a subflow */ MPTCP_MIB_MPPRIOTX, /* Transmit a MP_PRIO */ MPTCP_MIB_MPPRIORX, /* Received a MP_PRIO */ @@ -63,6 +73,14 @@ struct mptcp_mib { unsigned long mibs[LINUX_MIB_MPTCP_MAX]; }; +static inline void MPTCP_ADD_STATS(struct net *net, + enum linux_mptcp_mib_field field, + int val) +{ + if (likely(net->mib.mptcp_statistics)) + SNMP_ADD_STATS(net->mib.mptcp_statistics, field, val); +} + static inline void MPTCP_INC_STATS(struct net *net, enum linux_mptcp_mib_field field) { diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 19a01b6566f1..8a8083207be4 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -687,9 +687,12 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * } opts->suboptions |= OPTION_MPTCP_ADD_ADDR; if (!echo) { + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDRTX); opts->ahmac = add_addr_generate_hmac(msk->local_key, msk->remote_key, &opts->addr); + } else { + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADDTX); } pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); @@ -723,7 +726,7 @@ static bool mptcp_established_options_rm_addr(struct sock *sk, for (i = 0; i < opts->rm_list.nr; i++) pr_debug("rm_list_ids[%d]=%d", i, opts->rm_list.ids[i]); - + MPTCP_ADD_STATS(sock_net(sk), MPTCP_MIB_RMADDRTX, opts->rm_list.nr); return true; } diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 4ed4d29d9c11..7539137719ef 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -26,7 +26,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, if (add_addr & (echo ? BIT(MPTCP_ADD_ADDR_ECHO) : BIT(MPTCP_ADD_ADDR_SIGNAL))) { - pr_warn("addr_signal error, add_addr=%d, echo=%d", add_addr, echo); + MPTCP_INC_STATS(sock_net((struct sock *)msk), + echo ? MPTCP_MIB_ECHOADDTXDROP : MPTCP_MIB_ADDADDRTXDROP); return -EINVAL; } @@ -48,7 +49,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_ pr_debug("msk=%p, rm_list_nr=%d", msk, rm_list->nr); if (rm_addr) { - pr_warn("addr_signal error, rm_addr=%d", rm_addr); + MPTCP_ADD_STATS(sock_net((struct sock *)msk), + MPTCP_MIB_RMADDRTXDROP, rm_list->nr); return -EINVAL; } diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index fafd19ec7e1f..095ddd747ffc 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -1486,7 +1486,7 @@ chk_add_nr() fi echo -n " - echo " - count=$(ip netns exec $ns1 nstat -as | grep MPTcpExtEchoAdd | awk '{print $2}') + count=$(ip netns exec $ns1 nstat -as MPTcpExtEchoAdd | grep MPTcpExtEchoAdd | awk '{print $2}') [ -z "$count" ] && count=0 if [ "$count" != "$echo_nr" ]; then echo "[fail] got $count ADD_ADDR echo[s] expected $echo_nr" @@ -1608,7 +1608,7 @@ chk_rm_nr() fi printf "%-${nr_blank}s %s" " " "rm " - count=$(ip netns exec $addr_ns nstat -as | grep MPTcpExtRmAddr | awk '{print $2}') + count=$(ip netns exec $addr_ns nstat -as MPTcpExtRmAddr | grep MPTcpExtRmAddr | awk '{print $2}') [ -z "$count" ] && count=0 if [ "$count" != "$rm_addr_nr" ]; then echo "[fail] got $count RM_ADDR[s] expected $rm_addr_nr" -- 2.39.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs 2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni @ 2023-04-20 3:57 ` Geliang Tang 2023-04-20 10:18 ` Matthieu Baerts 2023-04-21 11:32 ` Matthieu Baerts 1 sibling, 1 reply; 11+ messages in thread From: Geliang Tang @ 2023-04-20 3:57 UTC (permalink / raw) To: Paolo Abeni; +Cc: mptcp Hi Paolo, How about also renaming AddAddr to AddAddrRx, and EchoAdd to EchoAddRx, RmAddr to RmAddrRx, RmAddrDrop to RmAddrRxDrop? Thanks, -Geliang Paolo Abeni <pabeni@redhat.com> 于2023年4月14日周五 22:19写道： > > Currently we don't track explicitly a few events related to address > management suboption handling; this patch adds new mibs for ADD_ADDR > and RM_ADDR options tx and for missed tx events due to internal storage > exahustion. > > The self-tests must be updated to properly handle different mibs with > the same/shared prefix. > > Additionally removes a couple of warning tracking the loss event. > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/378 > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > net/mptcp/mib.c | 6 ++++++ > net/mptcp/mib.h | 18 ++++++++++++++++++ > net/mptcp/options.c | 5 ++++- > net/mptcp/pm.c | 6 ++++-- > .../testing/selftests/net/mptcp/mptcp_join.sh | 4 ++-- > 5 files changed, 34 insertions(+), 5 deletions(-) > > diff --git a/net/mptcp/mib.c b/net/mptcp/mib.c > index 0dac2863c6e1..a0990c365a2e 100644 > --- a/net/mptcp/mib.c > +++ b/net/mptcp/mib.c > @@ -34,7 +34,11 @@ static const struct snmp_mib mptcp_snmp_list[] = { > SNMP_MIB_ITEM("NoDSSInWindow", MPTCP_MIB_NODSSWINDOW), > SNMP_MIB_ITEM("DuplicateData", MPTCP_MIB_DUPDATA), > SNMP_MIB_ITEM("AddAddr", MPTCP_MIB_ADDADDR), > + SNMP_MIB_ITEM("AddAddrTx", MPTCP_MIB_ADDADDRTX), > + SNMP_MIB_ITEM("AddAddrTxDrop", MPTCP_MIB_ADDADDRTXDROP), > SNMP_MIB_ITEM("EchoAdd", MPTCP_MIB_ECHOADD), > + SNMP_MIB_ITEM("EchoAddTx", MPTCP_MIB_ECHOADDTX), > + SNMP_MIB_ITEM("EchoAddTxDrop", MPTCP_MIB_ECHOADDTXDROP), > SNMP_MIB_ITEM("PortAdd", MPTCP_MIB_PORTADD), > SNMP_MIB_ITEM("AddAddrDrop", MPTCP_MIB_ADDADDRDROP), > SNMP_MIB_ITEM("MPJoinPortSynRx", MPTCP_MIB_JOINPORTSYNRX), > @@ -44,6 +48,8 @@ static const struct snmp_mib mptcp_snmp_list[] = { > SNMP_MIB_ITEM("MismatchPortAckRx", MPTCP_MIB_MISMATCHPORTACKRX), > SNMP_MIB_ITEM("RmAddr", MPTCP_MIB_RMADDR), > SNMP_MIB_ITEM("RmAddrDrop", MPTCP_MIB_RMADDRDROP), > + SNMP_MIB_ITEM("RmAddrTx", MPTCP_MIB_RMADDRTX), > + SNMP_MIB_ITEM("RmAddrTxDrop", MPTCP_MIB_RMADDRTXDROP), > SNMP_MIB_ITEM("RmSubflow", MPTCP_MIB_RMSUBFLOW), > SNMP_MIB_ITEM("MPPrioTx", MPTCP_MIB_MPPRIOTX), > SNMP_MIB_ITEM("MPPrioRx", MPTCP_MIB_MPPRIORX), > diff --git a/net/mptcp/mib.h b/net/mptcp/mib.h > index 2be3596374f4..cae71d947252 100644 > --- a/net/mptcp/mib.h > +++ b/net/mptcp/mib.h > @@ -27,7 +27,15 @@ enum linux_mptcp_mib_field { > MPTCP_MIB_NODSSWINDOW, /* Segments not in MPTCP windows */ > MPTCP_MIB_DUPDATA, /* Segments discarded due to duplicate DSS */ > MPTCP_MIB_ADDADDR, /* Received ADD_ADDR with echo-flag=0 */ > + MPTCP_MIB_ADDADDRTX, /* Sent ADD_ADDR with echo-flag=0 */ > + MPTCP_MIB_ADDADDRTXDROP, /* ADD_ADDR with echo-flag=0 not send due to > + * resource exhaustion > + */ > MPTCP_MIB_ECHOADD, /* Received ADD_ADDR with echo-flag=1 */ > + MPTCP_MIB_ECHOADDTX, /* Send ADD_ADDR with echo-flag=1 */ > + MPTCP_MIB_ECHOADDTXDROP, /* ADD_ADDR with echo-flag=1 not send due > + * to resource exhaustion > + */ > MPTCP_MIB_PORTADD, /* Received ADD_ADDR with a port-number */ > MPTCP_MIB_ADDADDRDROP, /* Dropped incoming ADD_ADDR */ > MPTCP_MIB_JOINPORTSYNRX, /* Received a SYN MP_JOIN with a different port-number */ > @@ -37,6 +45,8 @@ enum linux_mptcp_mib_field { > MPTCP_MIB_MISMATCHPORTACKRX, /* Received an ACK MP_JOIN with a mismatched port-number */ > MPTCP_MIB_RMADDR, /* Received RM_ADDR */ > MPTCP_MIB_RMADDRDROP, /* Dropped incoming RM_ADDR */ > + MPTCP_MIB_RMADDRTX, /* Sent RM_ADDR */ > + MPTCP_MIB_RMADDRTXDROP, /* RM_ADDR not sent due to resource exhaustion */ > MPTCP_MIB_RMSUBFLOW, /* Remove a subflow */ > MPTCP_MIB_MPPRIOTX, /* Transmit a MP_PRIO */ > MPTCP_MIB_MPPRIORX, /* Received a MP_PRIO */ > @@ -63,6 +73,14 @@ struct mptcp_mib { > unsigned long mibs[LINUX_MIB_MPTCP_MAX]; > }; > > +static inline void MPTCP_ADD_STATS(struct net *net, > + enum linux_mptcp_mib_field field, > + int val) > +{ > + if (likely(net->mib.mptcp_statistics)) > + SNMP_ADD_STATS(net->mib.mptcp_statistics, field, val); > +} > + > static inline void MPTCP_INC_STATS(struct net *net, > enum linux_mptcp_mib_field field) > { > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > index 19a01b6566f1..8a8083207be4 100644 > --- a/net/mptcp/options.c > +++ b/net/mptcp/options.c > @@ -687,9 +687,12 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > } > opts->suboptions |= OPTION_MPTCP_ADD_ADDR; > if (!echo) { > + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDRTX); > opts->ahmac = add_addr_generate_hmac(msk->local_key, > msk->remote_key, > &opts->addr); > + } else { > + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADDTX); > } > pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", > opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); > @@ -723,7 +726,7 @@ static bool mptcp_established_options_rm_addr(struct sock *sk, > > for (i = 0; i < opts->rm_list.nr; i++) > pr_debug("rm_list_ids[%d]=%d", i, opts->rm_list.ids[i]); > - > + MPTCP_ADD_STATS(sock_net(sk), MPTCP_MIB_RMADDRTX, opts->rm_list.nr); > return true; > } > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index 4ed4d29d9c11..7539137719ef 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -26,7 +26,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, > > if (add_addr & > (echo ? BIT(MPTCP_ADD_ADDR_ECHO) : BIT(MPTCP_ADD_ADDR_SIGNAL))) { > - pr_warn("addr_signal error, add_addr=%d, echo=%d", add_addr, echo); > + MPTCP_INC_STATS(sock_net((struct sock *)msk), > + echo ? MPTCP_MIB_ECHOADDTXDROP : MPTCP_MIB_ADDADDRTXDROP); > return -EINVAL; > } > > @@ -48,7 +49,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_ > pr_debug("msk=%p, rm_list_nr=%d", msk, rm_list->nr); > > if (rm_addr) { > - pr_warn("addr_signal error, rm_addr=%d", rm_addr); > + MPTCP_ADD_STATS(sock_net((struct sock *)msk), > + MPTCP_MIB_RMADDRTXDROP, rm_list->nr); > return -EINVAL; > } > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index fafd19ec7e1f..095ddd747ffc 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -1486,7 +1486,7 @@ chk_add_nr() > fi > > echo -n " - echo " > - count=$(ip netns exec $ns1 nstat -as | grep MPTcpExtEchoAdd | awk '{print $2}') > + count=$(ip netns exec $ns1 nstat -as MPTcpExtEchoAdd | grep MPTcpExtEchoAdd | awk '{print $2}') > [ -z "$count" ] && count=0 > if [ "$count" != "$echo_nr" ]; then > echo "[fail] got $count ADD_ADDR echo[s] expected $echo_nr" > @@ -1608,7 +1608,7 @@ chk_rm_nr() > fi > > printf "%-${nr_blank}s %s" " " "rm " > - count=$(ip netns exec $addr_ns nstat -as | grep MPTcpExtRmAddr | awk '{print $2}') > + count=$(ip netns exec $addr_ns nstat -as MPTcpExtRmAddr | grep MPTcpExtRmAddr | awk '{print $2}') > [ -z "$count" ] && count=0 > if [ "$count" != "$rm_addr_nr" ]; then > echo "[fail] got $count RM_ADDR[s] expected $rm_addr_nr" > -- > 2.39.2 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs 2023-04-20 3:57 ` Geliang Tang @ 2023-04-20 10:18 ` Matthieu Baerts 0 siblings, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2023-04-20 10:18 UTC (permalink / raw) To: Geliang Tang, Paolo Abeni; +Cc: mptcp Hi Geliang, On 20/04/2023 05:57, Geliang Tang wrote: > Hi Paolo, > > How about also renaming AddAddr to AddAddrRx, and EchoAdd to > EchoAddRx, RmAddr to RmAddrRx, RmAddrDrop to RmAddrRxDrop? I agree that it would be better but I don't think we can change the name. To me, it looks like we would break the user API: imagine apps/scripts relying on them, they would need to be adapted depending on the kernel version, not great. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs 2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni 2023-04-20 3:57 ` Geliang Tang @ 2023-04-21 11:32 ` Matthieu Baerts 1 sibling, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2023-04-21 11:32 UTC (permalink / raw) To: Paolo Abeni, mptcp Hi Paolo, On 14/04/2023 16:19, Paolo Abeni wrote: > Currently we don't track explicitly a few events related to address > management suboption handling; this patch adds new mibs for ADD_ADDR > and RM_ADDR options tx and for missed tx events due to internal storage > exahustion. (s/exahustion/exhaustion/) > The self-tests must be updated to properly handle different mibs with > the same/shared prefix. > > Additionally removes a couple of warning tracking the loss event. Thank you for having added these MIBs counters! I have one question below, just in case you need to send a v3. (...) > diff --git a/net/mptcp/options.c b/net/mptcp/options.c > index 19a01b6566f1..8a8083207be4 100644 > --- a/net/mptcp/options.c > +++ b/net/mptcp/options.c > @@ -687,9 +687,12 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff * > } > opts->suboptions |= OPTION_MPTCP_ADD_ADDR; > if (!echo) { > + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDRTX); > opts->ahmac = add_addr_generate_hmac(msk->local_key, > msk->remote_key, > &opts->addr); > + } else { > + MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADDTX); > } > pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d", > opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port)); > @@ -723,7 +726,7 @@ static bool mptcp_established_options_rm_addr(struct sock *sk, > > for (i = 0; i < opts->rm_list.nr; i++) > pr_debug("rm_list_ids[%d]=%d", i, opts->rm_list.ids[i]); > - (out of curiosity, why removing this line :) ) > + MPTCP_ADD_STATS(sock_net(sk), MPTCP_MIB_RMADDRTX, opts->rm_list.nr); > return true; > } > > diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c > index 4ed4d29d9c11..7539137719ef 100644 > --- a/net/mptcp/pm.c > +++ b/net/mptcp/pm.c > @@ -26,7 +26,8 @@ int mptcp_pm_announce_addr(struct mptcp_sock *msk, > > if (add_addr & > (echo ? BIT(MPTCP_ADD_ADDR_ECHO) : BIT(MPTCP_ADD_ADDR_SIGNAL))) { > - pr_warn("addr_signal error, add_addr=%d, echo=%d", add_addr, echo); > + MPTCP_INC_STATS(sock_net((struct sock *)msk), > + echo ? MPTCP_MIB_ECHOADDTXDROP : MPTCP_MIB_ADDADDRTXDROP); > return -EINVAL; > } > > @@ -48,7 +49,8 @@ int mptcp_pm_remove_addr(struct mptcp_sock *msk, const struct mptcp_rm_list *rm_ > pr_debug("msk=%p, rm_list_nr=%d", msk, rm_list->nr); > > if (rm_addr) { > - pr_warn("addr_signal error, rm_addr=%d", rm_addr); > + MPTCP_ADD_STATS(sock_net((struct sock *)msk), > + MPTCP_MIB_RMADDRTXDROP, rm_list->nr); > return -EINVAL; > } > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index fafd19ec7e1f..095ddd747ffc 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -1486,7 +1486,7 @@ chk_add_nr() > fi > > echo -n " - echo " > - count=$(ip netns exec $ns1 nstat -as | grep MPTcpExtEchoAdd | awk '{print $2}') > + count=$(ip netns exec $ns1 nstat -as MPTcpExtEchoAdd | grep MPTcpExtEchoAdd | awk '{print $2}') (we could also use "grep -w" but we did that kind of thing with nstat a few times in the past, no need to change) > [ -z "$count" ] && count=0 > if [ "$count" != "$echo_nr" ]; then > echo "[fail] got $count ADD_ADDR echo[s] expected $echo_nr" > @@ -1608,7 +1608,7 @@ chk_rm_nr() > fi > > printf "%-${nr_blank}s %s" " " "rm " > - count=$(ip netns exec $addr_ns nstat -as | grep MPTcpExtRmAddr | awk '{print $2}') > + count=$(ip netns exec $addr_ns nstat -as MPTcpExtRmAddr | grep MPTcpExtRmAddr | awk '{print $2}') > [ -z "$count" ] && count=0 > if [ "$count" != "$rm_addr_nr" ]; then > echo "[fail] got $count RM_ADDR[s] expected $rm_addr_nr" Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs 2023-04-14 14:19 [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Paolo Abeni 2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni @ 2023-04-14 14:19 ` Paolo Abeni 2023-04-21 11:32 ` Matthieu Baerts 2023-04-14 14:19 ` [PATCH v2 mptcp-next 3/3] selftests: mptcp: centralize stats dumping Paolo Abeni 2023-04-21 11:32 ` [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Matthieu Baerts 3 siblings, 1 reply; 11+ messages in thread From: Paolo Abeni @ 2023-04-14 14:19 UTC (permalink / raw) To: mptcp Instead of duplicating the all existing TX check with the TX side, add the new ones on selected test cases. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index 095ddd747ffc..f6ce10130ded 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -1579,6 +1579,44 @@ chk_add_nr() [ "${dump_stats}" = 1 ] && dump_stats } +chk_add_tx_nr() +{ + local add_tx_nr=$1 + local echo_tx_nr=$2 + local dump_stats + local timeout + local count + + timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout) + + printf "%-${nr_blank}s %s" " " "add TX" + count=$(ip netns exec $ns1 nstat -as MPTcpExtAddAddrTx | grep MPTcpExtAddAddrTx | awk '{print $2}') + [ -z "$count" ] && count=0 + + # if the test configured a short timeout tolerate greater then expected + # add addrs options, due to retransmissions + if [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_tx_nr" ]; }; then + echo "[fail] got $count ADD_ADDR[s] TX, expected $add_tx_nr" + fail_test + dump_stats=1 + else + echo -n "[ ok ]" + fi + + echo -n " - echo TX " + count=$(ip netns exec $ns2 nstat -as MPTcpExtEchoAddTx | grep MPTcpExtEchoAddTx | awk '{print $2}') + [ -z "$count" ] && count=0 + if [ "$count" != "$echo_tx_nr" ]; then + echo "[fail] got $count ADD_ADDR echo[s] TX, expected $echo_tx_nr" + fail_test + dump_stats=1 + else + echo "[ ok ]" + fi + + [ "${dump_stats}" = 1 ] && dump_stats +} + chk_rm_nr() { local rm_addr_nr=$1 @@ -1654,6 +1692,26 @@ chk_rm_nr() echo "$extra_msg" } +chk_rm_tx_nr() +{ + local rm_addr_tx_nr=$1 + + printf "%-${nr_blank}s %s" " " "rm TX " + count=$(ip netns exec $ns2 nstat -as MPTcpExtRmAddrTx | grep MPTcpExtRmAddrTx | awk '{print $2}') + [ -z "$count" ] && count=0 + if [ "$count" != "$rm_addr_tx_nr" ]; then + echo "[fail] got $count RM_ADDR[s] expected $rm_addr_tx_nr" + fail_test + dump_stats=1 + else + echo -n "[ ok ]" + fi + + [ "${dump_stats}" = 1 ] && dump_stats + + echo "$extra_msg" +} + chk_prio_nr() { local mp_prio_nr_tx=$1 @@ -1933,6 +1991,7 @@ signal_address_tests() pm_nl_add_endpoint $ns1 10.0.2.1 flags signal run_tests $ns1 $ns2 10.0.1.1 chk_join_nr 0 0 0 + chk_add_tx_nr 1 1 chk_add_nr 1 1 fi @@ -2114,6 +2173,7 @@ add_addr_timeout_tests() pm_nl_add_endpoint $ns1 10.0.2.1 flags signal run_tests $ns1 $ns2 10.0.1.1 0 0 0 slow chk_join_nr 1 1 1 + chk_add_tx_nr 4 4 chk_add_nr 4 0 fi @@ -2159,6 +2219,7 @@ remove_tests() pm_nl_add_endpoint $ns2 10.0.3.2 flags subflow run_tests $ns1 $ns2 10.0.1.1 0 0 -1 slow chk_join_nr 1 1 1 + chk_rm_tx_nr 1 chk_rm_nr 1 1 fi @@ -2257,6 +2318,7 @@ remove_tests() pm_nl_add_endpoint $ns2 10.0.4.2 flags subflow run_tests $ns1 $ns2 10.0.1.1 0 -8 -8 slow chk_join_nr 3 3 3 + chk_rm_tx_nr 0 chk_rm_nr 0 3 simult fi -- 2.39.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs 2023-04-14 14:19 ` [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs Paolo Abeni @ 2023-04-21 11:32 ` Matthieu Baerts 0 siblings, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2023-04-21 11:32 UTC (permalink / raw) To: Paolo Abeni, mptcp Hi Paolo, On 14/04/2023 16:19, Paolo Abeni wrote: > Instead of duplicating the all existing TX check with > the TX side, add the new ones on selected test cases. Good idea! > Signed-off-by: Paolo Abeni <pabeni@redhat.com> > --- > .../testing/selftests/net/mptcp/mptcp_join.sh | 62 +++++++++++++++++++ > 1 file changed, 62 insertions(+) > > diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh > index 095ddd747ffc..f6ce10130ded 100755 > --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh > +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh > @@ -1579,6 +1579,44 @@ chk_add_nr() > [ "${dump_stats}" = 1 ] && dump_stats > } > > +chk_add_tx_nr() > +{ > + local add_tx_nr=$1 > + local echo_tx_nr=$2 > + local dump_stats > + local timeout > + local count > + > + timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout) > + > + printf "%-${nr_blank}s %s" " " "add TX" > + count=$(ip netns exec $ns1 nstat -as MPTcpExtAddAddrTx | grep MPTcpExtAddAddrTx | awk '{print $2}') > + [ -z "$count" ] && count=0 In order to support running this on old kernels, could we check if the counter is available first and skip the checks if not? e.g. by using "nstat" with "-z" and return if '[ -z "${count}" ]'? We could also exit if there is no counter if ${SELFTESTS_MPTCP_LIB_EXTRA_CHECKS} is set, see: https://patchwork.kernel.org/project/mptcp/patch/20230406-mptcp-issue-368-selftests-old-kernels-v1-8-246ac567724d@tessares.net/ (We should add a helper to do this exit if the variable is set so we make sure this doesn't happen when our CI does the tests, always with the same version for the selftests and the kernel) > + > + # if the test configured a short timeout tolerate greater then expected > + # add addrs options, due to retransmissions > + if [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_tx_nr" ]; }; then > + echo "[fail] got $count ADD_ADDR[s] TX, expected $add_tx_nr" > + fail_test > + dump_stats=1 > + else > + echo -n "[ ok ]" > + fi > + > + echo -n " - echo TX " > + count=$(ip netns exec $ns2 nstat -as MPTcpExtEchoAddTx | grep MPTcpExtEchoAddTx | awk '{print $2}') > + [ -z "$count" ] && count=0 (if you use "nstat" with "-z", you don't need this line) > + if [ "$count" != "$echo_tx_nr" ]; then > + echo "[fail] got $count ADD_ADDR echo[s] TX, expected $echo_tx_nr" > + fail_test > + dump_stats=1 > + else > + echo "[ ok ]" > + fi > + > + [ "${dump_stats}" = 1 ] && dump_stats > +} > + > chk_rm_nr() > { > local rm_addr_nr=$1 > @@ -1654,6 +1692,26 @@ chk_rm_nr() > echo "$extra_msg" > } > > +chk_rm_tx_nr() > +{ > + local rm_addr_tx_nr=$1 > + > + printf "%-${nr_blank}s %s" " " "rm TX " > + count=$(ip netns exec $ns2 nstat -as MPTcpExtRmAddrTx | grep MPTcpExtRmAddrTx | awk '{print $2}') > + [ -z "$count" ] && count=0 Same here: skip the check if 'nstat -asz' return nothing? Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 mptcp-next 3/3] selftests: mptcp: centralize stats dumping 2023-04-14 14:19 [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Paolo Abeni 2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni 2023-04-14 14:19 ` [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs Paolo Abeni @ 2023-04-14 14:19 ` Paolo Abeni 2023-04-14 15:23 ` selftests: mptcp: centralize stats dumping: Tests Results MPTCP CI 2023-04-21 11:32 ` [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Matthieu Baerts 3 siblings, 1 reply; 11+ messages in thread From: Paolo Abeni @ 2023-04-14 14:19 UTC (permalink / raw) To: mptcp If a test case fails, the mptcp_join.sh script can dump the netns MIBs multiple times, leading to confusing output. Let's dump such info only once per test-case, when needed. This additionally allow removing some code duplication. Signed-off-by: Paolo Abeni <pabeni@redhat.com> --- .../testing/selftests/net/mptcp/mptcp_join.sh | 66 ++----------------- 1 file changed, 5 insertions(+), 61 deletions(-) diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh index f6ce10130ded..5ae2301407e2 100755 --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh @@ -30,6 +30,7 @@ evts_ns1="" evts_ns2="" evts_ns1_pid=0 evts_ns2_pid=0 +stats_dumped=0 declare -A all_tests declare -a only_tests_ids @@ -83,6 +84,7 @@ init_partial() fi done + stats_dumped=0 check_invert=0 validate_checksum=$checksum FAILING_LINKS="" @@ -343,6 +345,9 @@ fail_test() { ret=1 failed_tests[${TEST_COUNT}]="${TEST_NAME}" + + [ "${stats_dumped}" = 0 ] && dump_stats + stats_dumped=1 } get_failed_tests_ids() @@ -1114,7 +1119,6 @@ chk_csum_nr() local csum_ns1=${1:-0} local csum_ns2=${2:-0} local count - local dump_stats local extra_msg="" local allow_multi_errors_ns1=0 local allow_multi_errors_ns2=0 @@ -1138,7 +1142,6 @@ chk_csum_nr() { [ "$count" -lt $csum_ns1 ] && [ $allow_multi_errors_ns1 -eq 1 ]; }; then echo "[fail] got $count data checksum error[s] expected $csum_ns1" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1152,11 +1155,9 @@ chk_csum_nr() { [ "$count" -lt $csum_ns2 ] && [ $allow_multi_errors_ns2 -eq 1 ]; }; then echo "[fail] got $count data checksum error[s] expected $csum_ns2" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats echo "$extra_msg" } @@ -1167,7 +1168,6 @@ chk_fail_nr() local fail_rx=$2 local ns_invert=${3:-""} local count - local dump_stats local ns_tx=$ns1 local ns_rx=$ns2 local extra_msg="" @@ -1199,7 +1199,6 @@ chk_fail_nr() { [ "$count" -gt "$fail_tx" ] && [ $allow_tx_lost -eq 1 ]; }; then echo "[fail] got $count MP_FAIL[s] TX expected $fail_tx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1214,13 +1213,10 @@ chk_fail_nr() { [ "$count" -gt "$fail_rx" ] && [ $allow_rx_lost -eq 1 ]; }; then echo "[fail] got $count MP_FAIL[s] RX expected $fail_rx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats - echo "$extra_msg" } @@ -1230,7 +1226,6 @@ chk_fclose_nr() local fclose_rx=$2 local ns_invert=$3 local count - local dump_stats local ns_tx=$ns2 local ns_rx=$ns1 local extra_msg=" " @@ -1248,7 +1243,6 @@ chk_fclose_nr() if [ "$count" != "$fclose_tx" ]; then echo "[fail] got $count MP_FASTCLOSE[s] TX expected $fclose_tx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1260,13 +1254,10 @@ chk_fclose_nr() if [ "$count" != "$fclose_rx" ]; then echo "[fail] got $count MP_FASTCLOSE[s] RX expected $fclose_rx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats - echo "$extra_msg" } @@ -1276,7 +1267,6 @@ chk_rst_nr() local rst_rx=$2 local ns_invert=${3:-""} local count - local dump_stats local ns_tx=$ns1 local ns_rx=$ns2 local extra_msg="" @@ -1293,7 +1283,6 @@ chk_rst_nr() if [ $count -lt $rst_tx ]; then echo "[fail] got $count MP_RST[s] TX expected $rst_tx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1304,13 +1293,10 @@ chk_rst_nr() if [ "$count" -lt "$rst_rx" ]; then echo "[fail] got $count MP_RST[s] RX expected $rst_rx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats - echo "$extra_msg" } @@ -1319,7 +1305,6 @@ chk_infi_nr() local infi_tx=$1 local infi_rx=$2 local count - local dump_stats printf "%-${nr_blank}s %s" " " "itx" count=$(ip netns exec $ns2 nstat -as | grep InfiniteMapTx | awk '{print $2}') @@ -1327,7 +1312,6 @@ chk_infi_nr() if [ "$count" != "$infi_tx" ]; then echo "[fail] got $count infinite map[s] TX expected $infi_tx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1338,12 +1322,9 @@ chk_infi_nr() if [ "$count" != "$infi_rx" ]; then echo "[fail] got $count infinite map[s] RX expected $infi_rx" fail_test - dump_stats=1 else echo "[ ok ]" fi - - [ "${dump_stats}" = 1 ] && dump_stats } chk_join_nr() @@ -1358,7 +1339,6 @@ chk_join_nr() local infi_nr=${8:-0} local corrupted_pkts=${9:-0} local count - local dump_stats local with_cookie local title="${TEST_NAME}" @@ -1372,7 +1352,6 @@ chk_join_nr() if [ "$count" != "$syn_nr" ]; then echo "[fail] got $count JOIN[s] syn expected $syn_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1390,7 +1369,6 @@ chk_join_nr() else echo "[fail] got $count JOIN[s] synack expected $syn_ack_nr" fail_test - dump_stats=1 fi else echo -n "[ ok ]" @@ -1402,11 +1380,9 @@ chk_join_nr() if [ "$count" != "$ack_nr" ]; then echo "[fail] got $count JOIN[s] ack expected $ack_nr" fail_test - dump_stats=1 else echo "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats if [ $validate_checksum -eq 1 ]; then chk_csum_nr $csum_ns1 $csum_ns2 chk_fail_nr $fail_nr $fail_nr @@ -1466,7 +1442,6 @@ chk_add_nr() local mis_syn_nr=${7:-0} local mis_ack_nr=${8:-0} local count - local dump_stats local timeout timeout=$(ip netns exec $ns1 sysctl -n net.mptcp.add_addr_timeout) @@ -1480,7 +1455,6 @@ chk_add_nr() if [ "$count" != "$add_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_nr" ]; }; then echo "[fail] got $count ADD_ADDR[s] expected $add_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1491,7 +1465,6 @@ chk_add_nr() if [ "$count" != "$echo_nr" ]; then echo "[fail] got $count ADD_ADDR echo[s] expected $echo_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1503,7 +1476,6 @@ chk_add_nr() if [ "$count" != "$port_nr" ]; then echo "[fail] got $count ADD_ADDR[s] with a port-number expected $port_nr" fail_test - dump_stats=1 else echo "[ ok ]" fi @@ -1516,7 +1488,6 @@ chk_add_nr() echo "[fail] got $count JOIN[s] syn with a different \ port-number expected $syn_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1529,7 +1500,6 @@ chk_add_nr() echo "[fail] got $count JOIN[s] synack with a different \ port-number expected $syn_ack_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1542,7 +1512,6 @@ chk_add_nr() echo "[fail] got $count JOIN[s] ack with a different \ port-number expected $ack_nr" fail_test - dump_stats=1 else echo "[ ok ]" fi @@ -1555,7 +1524,6 @@ chk_add_nr() echo "[fail] got $count JOIN[s] syn with a mismatched \ port-number expected $mis_syn_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1568,22 +1536,18 @@ chk_add_nr() echo "[fail] got $count JOIN[s] ack with a mismatched \ port-number expected $mis_ack_nr" fail_test - dump_stats=1 else echo "[ ok ]" fi else echo "" fi - - [ "${dump_stats}" = 1 ] && dump_stats } chk_add_tx_nr() { local add_tx_nr=$1 local echo_tx_nr=$2 - local dump_stats local timeout local count @@ -1598,7 +1562,6 @@ chk_add_tx_nr() if [ "$count" != "$add_tx_nr" ] && { [ "$timeout" -gt 1 ] || [ "$count" -lt "$add_tx_nr" ]; }; then echo "[fail] got $count ADD_ADDR[s] TX, expected $add_tx_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1609,12 +1572,9 @@ chk_add_tx_nr() if [ "$count" != "$echo_tx_nr" ]; then echo "[fail] got $count ADD_ADDR echo[s] TX, expected $echo_tx_nr" fail_test - dump_stats=1 else echo "[ ok ]" fi - - [ "${dump_stats}" = 1 ] && dump_stats } chk_rm_nr() @@ -1624,7 +1584,6 @@ chk_rm_nr() local invert local simult local count - local dump_stats local addr_ns=$ns1 local subflow_ns=$ns2 local extra_msg="" @@ -1651,7 +1610,6 @@ chk_rm_nr() if [ "$count" != "$rm_addr_nr" ]; then echo "[fail] got $count RM_ADDR[s] expected $rm_addr_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1675,20 +1633,16 @@ chk_rm_nr() else echo "[fail] got $count RM_SUBFLOW[s] expected in range [$rm_subflow_nr:$((rm_subflow_nr*2))]" fail_test - dump_stats=1 fi return fi if [ "$count" != "$rm_subflow_nr" ]; then echo "[fail] got $count RM_SUBFLOW[s] expected $rm_subflow_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats - echo "$extra_msg" } @@ -1702,13 +1656,10 @@ chk_rm_tx_nr() if [ "$count" != "$rm_addr_tx_nr" ]; then echo "[fail] got $count RM_ADDR[s] expected $rm_addr_tx_nr" fail_test - dump_stats=1 else echo -n "[ ok ]" fi - [ "${dump_stats}" = 1 ] && dump_stats - echo "$extra_msg" } @@ -1717,7 +1668,6 @@ chk_prio_nr() local mp_prio_nr_tx=$1 local mp_prio_nr_rx=$2 local count - local dump_stats printf "%-${nr_blank}s %s" " " "ptx" count=$(ip netns exec $ns1 nstat -as | grep MPTcpExtMPPrioTx | awk '{print $2}') @@ -1725,7 +1675,6 @@ chk_prio_nr() if [ "$count" != "$mp_prio_nr_tx" ]; then echo "[fail] got $count MP_PRIO[s] TX expected $mp_prio_nr_tx" fail_test - dump_stats=1 else echo -n "[ ok ]" fi @@ -1736,12 +1685,9 @@ chk_prio_nr() if [ "$count" != "$mp_prio_nr_rx" ]; then echo "[fail] got $count MP_PRIO[s] RX expected $mp_prio_nr_rx" fail_test - dump_stats=1 else echo "[ ok ]" fi - - [ "${dump_stats}" = 1 ] && dump_stats } chk_subflow_nr() @@ -1773,7 +1719,6 @@ chk_subflow_nr() ss -N $ns1 -tOni ss -N $ns1 -tOni | grep token ip -n $ns1 mptcp endpoint - dump_stats fi } @@ -1813,7 +1758,6 @@ chk_mptcp_info() if [ "$dump_stats" = 1 ]; then ss -N $ns1 -inmHM ss -N $ns2 -inmHM - dump_stats fi } -- 2.39.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: selftests: mptcp: centralize stats dumping: Tests Results 2023-04-14 14:19 ` [PATCH v2 mptcp-next 3/3] selftests: mptcp: centralize stats dumping Paolo Abeni @ 2023-04-14 15:23 ` MPTCP CI 0 siblings, 0 replies; 11+ messages in thread From: MPTCP CI @ 2023-04-14 15:23 UTC (permalink / raw) To: Paolo Abeni; +Cc: mptcp Hi Paolo, Thank you for your modifications, that's great! Our CI did some validations and here is its report: - KVM Validation: normal (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5301653952266240 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5301653952266240/summary/summary.txt - KVM Validation: normal (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/6427553859108864 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6427553859108864/summary/summary.txt - KVM Validation: debug (except selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/5020178975555584 - Summary: https://api.cirrus-ci.com/v1/artifact/task/5020178975555584/summary/summary.txt - KVM Validation: debug (only selftest_mptcp_join): - Success! ✅: - Task: https://cirrus-ci.com/task/6146078882398208 - Summary: https://api.cirrus-ci.com/v1/artifact/task/6146078882398208/summary/summary.txt Initiator: Patchew Applier Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/d17c54b50423 If there are some issues, you can reproduce them using the same environment as the one used by the CI thanks to a docker image, e.g.: $ cd [kernel source code] $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \ --pull always mptcp/mptcp-upstream-virtme-docker:latest \ auto-debug For more details: https://github.com/multipath-tcp/mptcp-upstream-virtme-docker Please note that despite all the efforts that have been already done to have a stable tests suite when executed on a public CI like here, it is possible some reported issues are not due to your modifications. Still, do not hesitate to help us improve that ;-) Cheers, MPTCP GH Action bot Bot operated by Matthieu Baerts (Tessares) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 0/3] mptcp: add more mibs 2023-04-14 14:19 [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Paolo Abeni ` (2 preceding siblings ...) 2023-04-14 14:19 ` [PATCH v2 mptcp-next 3/3] selftests: mptcp: centralize stats dumping Paolo Abeni @ 2023-04-21 11:32 ` Matthieu Baerts 2023-05-03 12:36 ` Matthieu Baerts 3 siblings, 1 reply; 11+ messages in thread From: Matthieu Baerts @ 2023-04-21 11:32 UTC (permalink / raw) To: Paolo Abeni, mptcp Hi Paolo, On 14/04/2023 16:19, Paolo Abeni wrote: > dumb cover letter for CI's sake. > > Slould be applied only after the current tree has been crunched > by syzkaller for a little bit, as this will likely change quite > a bit the timings of some critical path, possibly hiding old races > and make more easy to reach other ones > > Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/378 Thank you for having looked at that! I have a small request in patch 2/3 (and a question in 1/3 but that's a detail) if you don't mind. We can also apply the patches and do the suggested improvement after if you prefer. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 mptcp-next 0/3] mptcp: add more mibs 2023-04-21 11:32 ` [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Matthieu Baerts @ 2023-05-03 12:36 ` Matthieu Baerts 0 siblings, 0 replies; 11+ messages in thread From: Matthieu Baerts @ 2023-05-03 12:36 UTC (permalink / raw) To: Paolo Abeni, mptcp Hi Paolo, On 21/04/2023 13:32, Matthieu Baerts wrote: > Hi Paolo, > > On 14/04/2023 16:19, Paolo Abeni wrote: >> dumb cover letter for CI's sake. >> >> Slould be applied only after the current tree has been crunched >> by syzkaller for a little bit, as this will likely change quite >> a bit the timings of some critical path, possibly hiding old races >> and make more easy to reach other ones >> >> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/378 > > Thank you for having looked at that! > > I have a small request in patch 2/3 (and a question in 1/3 but that's a > detail) if you don't mind. > > We can also apply the patches and do the suggested improvement after if > you prefer. As discussed at the last weekly meeting, I just applied the patches in our tree (feat. for net-next) with my RvB (and without a typo): New patches for t/upstream: - 11f98962ef03: mptcp: introduces more address related mibs - ae2301fadbf9: selftests: mptcp: add explicit check for new mibs - 3001ad5baef7: selftests: mptcp: centralize stats dumping - Results: 269bb6df6e7f..b2cd747b2dff (export) Tests are now in progress: https://cirrus-ci.com/github/multipath-tcp/mptcp_net-next/export/20230503T123221 Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-03 12:36 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-14 14:19 [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Paolo Abeni 2023-04-14 14:19 ` [PATCH v2 mptcp-next 1/3] mptcp: introduces more address related mibs Paolo Abeni 2023-04-20 3:57 ` Geliang Tang 2023-04-20 10:18 ` Matthieu Baerts 2023-04-21 11:32 ` Matthieu Baerts 2023-04-14 14:19 ` [PATCH v2 mptcp-next 2/3] selftests: mptcp: add explicit check for new mibs Paolo Abeni 2023-04-21 11:32 ` Matthieu Baerts 2023-04-14 14:19 ` [PATCH v2 mptcp-next 3/3] selftests: mptcp: centralize stats dumping Paolo Abeni 2023-04-14 15:23 ` selftests: mptcp: centralize stats dumping: Tests Results MPTCP CI 2023-04-21 11:32 ` [PATCH v2 mptcp-next 0/3] mptcp: add more mibs Matthieu Baerts 2023-05-03 12:36 ` Matthieu Baerts
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.