| Message ID | 20240417085143.69578-2-kerneljasonxing@gmail.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Implement reset reason mechanism to detect | expand |
| Context | Check | Description |
|---|---|---|
| matttbe/checkpatch | fail | total: 1 errors, 1 warnings, 1 checks, 93 lines checked |
| matttbe/shellcheck | success | MPTCP selftests files have not been modified |
| matttbe/build | success | Build and static analysis OK |
| matttbe/KVM_Validation__normal | success | Success! ✅ |
| matttbe/KVM_Validation__debug | warning | Unstable: 2 failed test(s): packetdrill_dss packetdrill_fastopen |
| matttbe/KVM_Validation__btf__only_bpftest_all_ | success | Success! ✅ |
On Wed, Apr 17, 2024 at 10:51 AM Jason Xing <kerneljasonxing@gmail.com> wrote: > > From: Jason Xing <kernelxing@tencent.com> > > Add a new standalone file for the easy future extension to support > both active reset and passive reset in the TCP/DCCP/MPTCP protocols. > > This patch only does the preparations for reset reason mechanism, > nothing else changes. > > The reset reasons are divided into three parts: > 1) reuse drop reasons for passive reset in TCP > 2) reuse MP_TCPRST option for MPTCP > 3) our own reasons > > I will implement the basic codes of active/passive reset reason in > those three protocols, which is not complete for this moment. But > it provides a new chance to let other people add more reasons into > it:) > > Signed-off-by: Jason Xing <kernelxing@tencent.com> My original suggestion was to use normal values in 'enum skb_drop_reason', even if there was not necessarily a 'drop' in the common sense. https://lore.kernel.org/all/CANn89iJw8x-LqgsWOeJQQvgVg6DnL5aBRLi10QN2WBdr+X4k=w@mail.gmail.com/ This would avoid these ugly casts later, even casting an enum to other ones is not very logical. Going through an u32 pivot is quite a hack. If you feel the need to put them in a special group, this is fine by me.
Hello Eric, On Wed, Apr 17, 2024 at 5:02 PM Eric Dumazet <edumazet@google.com> wrote: > > On Wed, Apr 17, 2024 at 10:51 AM Jason Xing <kerneljasonxing@gmail.com> wrote: > > > > From: Jason Xing <kernelxing@tencent.com> > > > > Add a new standalone file for the easy future extension to support > > both active reset and passive reset in the TCP/DCCP/MPTCP protocols. > > > > This patch only does the preparations for reset reason mechanism, > > nothing else changes. > > > > The reset reasons are divided into three parts: > > 1) reuse drop reasons for passive reset in TCP > > 2) reuse MP_TCPRST option for MPTCP > > 3) our own reasons > > > > I will implement the basic codes of active/passive reset reason in > > those three protocols, which is not complete for this moment. But > > it provides a new chance to let other people add more reasons into > > it:) > > > > Signed-off-by: Jason Xing <kernelxing@tencent.com> > > My original suggestion was to use normal values in 'enum > skb_drop_reason', even if there was not necessarily a 'drop' > in the common sense. > > https://lore.kernel.org/all/CANn89iJw8x-LqgsWOeJQQvgVg6DnL5aBRLi10QN2WBdr+X4k=w@mail.gmail.com/ > > This would avoid these ugly casts later, even casting an enum to other > ones is not very logical. Thanks for your comment. It's a little bit tricky. That's the reason I documented and commented on this in the rstreason.h file. I hope it's not that hard to understand. > Going through an u32 pivot is quite a hack. > > If you feel the need to put them in a special group, this is fine by me. Yes, rst reasons only partially rely on the drop reason mechanism to support passive rst for TCP well, but not supporting other cases. My final goal is to cover all the cases for the future, so I wish I can put it into a separate group, then people like me who find it useful can introduce more reasons into it. Thanks, Jason
diff --git a/include/net/rstreason.h b/include/net/rstreason.h new file mode 100644 index 000000000000..0c3fa55fa62f --- /dev/null +++ b/include/net/rstreason.h @@ -0,0 +1,93 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#ifndef _LINUX_RSTREASON_H +#define _LINUX_RSTREASON_H +#include <net/dropreason-core.h> + +#define DEFINE_RST_REASON(FN, FNe) \ + FN(MPTCP_RST_EUNSPEC) \ + FN(MPTCP_RST_EMPTCP) \ + FN(MPTCP_RST_ERESOURCE) \ + FN(MPTCP_RST_EPROHIBIT) \ + FN(MPTCP_RST_EWQ2BIG) \ + FN(MPTCP_RST_EBADPERF) \ + FN(MPTCP_RST_EMIDDLEBOX) \ + FN(NOT_SPECIFIED) \ + FNe(MAX) + +#define RST_REASON_START (SKB_DROP_REASON_MAX + 1) + +/* There are three parts in order: + * 1) 0 - SKB_DROP_REASON_MAX: rely on drop reasons for passive reset in TCP + * 2) SKB_DROP_REASON_MAX + 1 - MPTCP_RST_EMIDDLEBOX: for MPTCP use + * 3) MPTCP_RST_EMIDDLEBOX - SK_RST_REASON_MAX: independent reset reason + */ +enum sk_rst_reason { + /* Leave this 'blank' part (0-SKB_DROP_REASON_MAX) for the reuse + * of skb drop reason because rst reason relies on what drop reason + * indicates exactly why it could happen. + */ + + /* Copy from include/uapi/linux/mptcp.h. + * These reset fields will not be changed since they adhere to + * RFC 8684. So do not touch them. I'm going to list each definition + * of them respectively. + */ + /* Unspecified error. + * This is the default error; it implies that the subflow is no + * longer available. The presence of this option shows that the + * RST was generated by an MPTCP-aware device. + */ + SK_RST_REASON_MPTCP_RST_EUNSPEC = RST_REASON_START, + /* MPTCP-specific error. + * An error has been detected in the processing of MPTCP options. + * This is the usual reason code to return in the cases where a RST + * is being sent to close a subflow because of an invalid response. + */ + SK_RST_REASON_MPTCP_RST_EMPTCP, + /* Lack of resources. + * This code indicates that the sending host does not have enough + * resources to support the terminated subflow. + */ + SK_RST_REASON_MPTCP_RST_ERESOURCE, + /* Administratively prohibited. + * This code indicates that the requested subflow is prohibited by + * the policies of the sending host. + */ + SK_RST_REASON_MPTCP_RST_EPROHIBIT, + /* Too much outstanding data. + * This code indicates that there is an excessive amount of data + * that needs to be transmitted over the terminated subflow while + * having already been acknowledged over one or more other subflows. + * This may occur if a path has been unavailable for a short period + * and it is more efficient to reset and start again than it is to + * retransmit the queued data. + */ + SK_RST_REASON_MPTCP_RST_EWQ2BIG, + /* Unacceptable performance. + * This code indicates that the performance of this subflow was + * too low compared to the other subflows of this Multipath TCP + * connection. + */ + SK_RST_REASON_MPTCP_RST_EBADPERF, + /* Middlebox interference. + * Middlebox interference has been detected over this subflow, + * making MPTCP signaling invalid. For example, this may be sent + * if the checksum does not validate. + */ + SK_RST_REASON_MPTCP_RST_EMIDDLEBOX, + + /* For the real standalone socket reset reason, we start from here */ + SK_RST_REASON_NOT_SPECIFIED, + + /* Maximum of socket reset reasons. + * It shouldn't be used as a real 'reason'. + */ + SK_RST_REASON_MAX, +}; + +static inline enum sk_rst_reason convert_mptcp_reason(u32 reason) +{ + return reason += RST_REASON_START; +} +#endif