Hi Christoph, On Mon, 30 Oct 2017, Christoph Paasch wrote: > On 30/10/17 - 15:44:03, Mat Martineau wrote: >> >> On Sun, 29 Oct 2017, Christoph Paasch wrote: >> >>> How do you want to proceed for submitting it upstream? I fear that >>> without a >>> user of skb_shared_info_ext, netdev won't accept it. >>> >>> Shouldn't we try to find a user that would benefit from it? >> >> I was expecting to wait for an MPTCP patch set that would make use of the >> shared control block, but I agree it would be better to find an existing >> struct sk_buff user that would benefit. I'll look around for a protocol >> with >> a crowded skb->cb - there could be some cases where there are painful work >> arounds for the size constraints. > > Yes, having another user of it is definitely better. I looked a bit through > the code, but couldn't find a good use-case though. The requirements are fairly narrow to use the shared cb, which makes it hard to use outside MPTCP: * Only tx skbs (rx skbs allocated by a driver don't know when to add the extra space) * Data in the shared cb is read-only (or safely shared) * Other users don't conflict with MPTCP The closest I've found so far is dccp_skb_cb... but: We could do something with __build_skb() instead, allocating our own data with extra bytes at the end. I think a bit in tcp_skb_cb would still be necessary so the header writing code could tell if the extended MPTCP cb is present. The size of the extra data is more flexible this way, since it's not baked in to the common skb code. Zero changes to the global skb code is very compelling and makes it easier to upstream. Thoughts?

Hullo, On 10/23/2017 04:10 PM, Mat Martineau wrote: > > Yes, I'm interested too! I know David Miller is interested in making > good use of space in data structures: > https://netdevconf.org/2.2/session.html?miller-datastructurebloat-keynote > > > If there's a lot of unnecessary space being used they'd probably want > to reclaim it and then we'd need the extra shared info again :) > Thanks for the pointer, I will take a look. Take a look at my last couple of patches, they are all using regular size CB. The relevant changes are in mptcp_skb_entail() and mptcp_write_dss_mapping() and the obvious  tcp_skb_cb and sk_buff structures. The change is to calculate the mapping when the header is being written not before. I am working on net-next 4.14.0-rc4 and have been able to achieve the same. Here are the changes to the structure, cut and paste of the relevant code from any patch just works. The change to tcp_skb_cb is         __u8            ip_dsfield;     /* IPv4 tos or IPv6 dsfield     */         __u8            txstamp_ack:1,  /* Record TX timestamp for ack? */                         eor:1,          /* Is skb MSG_EOR marked? */                         has_rxtstamp:1, /* SKB has a RX timestamp       */                         unused:5;         union {                         __u32   ack_seq;        /* Sequence number ACK'd */                         __u32   mptcp_data_seq;                         __u32   mptcp_path_mask;         }; And used a scratch field in sk_buff, these are only needed on the Rx side. struct sk_buff {         union {                 struct {                         /* These two members must be first. */                         struct sk_buff          *next;                         struct sk_buff          *prev;                         union {                                 struct net_device       *dev;                                 /* Some protocols might use this space to store information,                                  * while device pointer would be NULL.                                  * UDP receive path is one user.                                  */                                 unsigned long dev_scratch;                                 struct {                                         __u8 mptcp_flags;                                         __u8 mptcp_dss_off;                                 };                         };                 };                 struct rb_node  rbnode; /* used in netem & tcp stack */         }; And here is a tcpdump. Host 192.168.1.3 is running net-next "4.14.0 Fearless Coyote" and host 192.168.1.32 is running "4.4.88 Blurry Fish Butt" 15:21:28.581388 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [S], seq 1515497212, win 29200, options [mss 1460,sackOK,TS val 2460468540 ecr 0,nop,wscale 7,mptcp capable csum {0x26b923d415f6c4d4}], length 0 15:21:28.581449 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [S.], seq 3412665074, ack 1515497213, win 28560, options [mss 1460,sackOK,TS val 25517800 ecr 2460468540,nop,wscale 7,mptcp capable csum {0x8fd2dd8f92b5f72c}], length 0 15:21:28.582533 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1:42, ack 1, win 229, options [nop,nop,TS val 2460468541 ecr 25517800], length 41 15:21:28.582612 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 42, win 224, options [nop,nop,TS val 25517800 ecr 2460468541], length 0 15:21:28.591889 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1:42, ack 42, win 224, options [nop,nop,TS val 25517802 ecr 2460468541], length 41 15:21:28.592922 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 42:1378, ack 42, win 229, options [nop,nop,TS val 2460468552 ecr 25517802], length 1336 15:21:28.593028 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 42:1018, ack 42, win 224, options [nop,nop,TS val 25517803 ecr 2460468552], length 976 15:21:28.633043 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1378, win 246, options [nop,nop,TS val 25517813 ecr 2460468552], length 0 15:21:28.633956 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1378:1426, ack 1018, win 244, options [nop,nop,TS val 2460468593 ecr 25517803], length 48 15:21:28.634012 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1426, win 246, options [nop,nop,TS val 25517813 ecr 2460468593], length 0 15:21:28.647684 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1018:1382, ack 1426, win 246, options [nop,nop,TS val 25517816 ecr 2460468593], length 364 15:21:28.653439 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1426:1442, ack 1382, win 259, options [nop,nop,TS val 2460468612 ecr 25517816], length 16 15:21:28.692996 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1442, win 246, options [nop,nop,TS val 25517828 ecr 2460468612], length 0 15:21:28.693584 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1442:1486, ack 1382, win 259, options [nop,nop,TS val 2460468652 ecr 25517828], length 44 15:21:28.693632 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1486, win 246, options [nop,nop,TS val 25517828 ecr 2460468652], length 0 15:21:28.693706 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1382:1426, ack 1486, win 246, options [nop,nop,TS val 25517828 ecr 2460468652], length 44 15:21:28.694310 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1486:1554, ack 1426, win 259, options [nop,nop,TS val 2460468653 ecr 25517828], length 68 15:21:28.694976 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1426:1478, ack 1554, win 246, options [nop,nop,TS val 25517828 ecr 2460468653], length 52 15:21:28.737617 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [.], ack 1478, win 259, options [nop,nop,TS val 2460468654 ecr 25517828], length 0 15:21:31.599860 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1554:1702, ack 1478, win 259, options [nop,nop,TS val 2460471559 ecr 25517828], length 148 I have some other ideas that I want to try but right now I focused on getting something working so we can all start hacking. Rao >> I had to bump cb up to 80 bytes in the current mptcp_trunk :/ >> >> >> Christoph >> >>> FYI. I am working on restructuring MPTCP code so it's not intrusive >>> to main >>> TCP code. I will probably try a few other techniques but can use >>> this as >>> well. I will post the changes at the appropriate time. > > Nice, thanks for the update. > > Regards, > Mat _______________________________________________ mptcp mailing list mptcp(a)lists.01.org https://lists.01.org/mailman/listinfo/mptcp

Hullo, On 10/23/2017 04:10 PM, Mat Martineau wrote: > > Yes, I'm interested too! I know David Miller is interested in making > good use of space in data structures: > https://netdevconf.org/2.2/session.html?miller-datastructurebloat-keynote > > If there's a lot of unnecessary space being used they'd probably want to > reclaim it and then we'd need the extra shared info again :) > Thanks for the pointer, I will take a look. Take a look at my last couple of patches, they are all using regular size CB. The relevant changes are in mptcp_skb_entail() and mptcp_write_dss_mapping() and the obvious  tcp_skb_cb and sk_buff structures. The change is to calculate the mapping when the header is being written not before. I am working on net-next 4.14.0-rc4 and have been able to achieve the same. Here are the changes to the structure, cut and paste of the relevant code from any patch just works. The change to tcp_skb_cb is         __u8            ip_dsfield;     /* IPv4 tos or IPv6 dsfield     */         __u8            txstamp_ack:1,  /* Record TX timestamp for ack? */                         eor:1,          /* Is skb MSG_EOR marked? */                         has_rxtstamp:1, /* SKB has a RX timestamp       */                         unused:5;         union {                         __u32   ack_seq;        /* Sequence number ACK'd */                         __u32   mptcp_data_seq;                         __u32   mptcp_path_mask;         }; And used a scratch field in sk_buff, these are only needed on the Rx side.

struct sk_buff {         union {                 struct {                         /* These two members must be first. */                         struct sk_buff          *next;                         struct sk_buff          *prev;                         union {                                 struct net_device       *dev;                                 /* Some protocols might use this space to store information,                                  * while device pointer would be NULL.                                  * UDP receive path is one user.                                  */                                 unsigned long           dev_scratch;                                 struct {                                         __u8 mptcp_flags;                                         __u8 mptcp_dss_off;                                 };                         };                 };                 struct rb_node  rbnode; /* used in netem & tcp stack */         }; And here is a tcpdump. Host 192.168.1.3 is running net-next "4.14.0 Fearless Coyote" and host 192.168.1.32 is running "4.4.88 Blurry Fish Butt" 15:21:28.581388 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [S], seq 1515497212, win 29200, options [mss 1460,sackOK,TS val 2460468540 ecr 0,nop,wscale 7,mptcp capable csum {0x26b923d415f6c4d4}], length 0 15:21:28.581449 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [S.], seq 3412665074, ack 1515497213, win 28560, options [mss 1460,sackOK,TS val 25517800 ecr 2460468540,nop,wscale 7,mptcp capable csum {0x8fd2dd8f92b5f72c}], length 0 15:21:28.582533 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1:42, ack 1, win 229, options [nop,nop,TS val 2460468541 ecr 25517800], length 41 15:21:28.582612 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 42, win 224, options [nop,nop,TS val 25517800 ecr 2460468541], length 0 15:21:28.591889 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1:42, ack 42, win 224, options [nop,nop,TS val 25517802 ecr 2460468541], length 41 15:21:28.592922 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 42:1378, ack 42, win 229, options [nop,nop,TS val 2460468552 ecr 25517802], length 1336 15:21:28.593028 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 42:1018, ack 42, win 224, options [nop,nop,TS val 25517803 ecr 2460468552], length 976 15:21:28.633043 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1378, win 246, options [nop,nop,TS val 25517813 ecr 2460468552], length 0 15:21:28.633956 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1378:1426, ack 1018, win 244, options [nop,nop,TS val 2460468593 ecr 25517803], length 48 15:21:28.634012 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1426, win 246, options [nop,nop,TS val 25517813 ecr 2460468593], length 0 15:21:28.647684 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1018:1382, ack 1426, win 246, options [nop,nop,TS val 25517816 ecr 2460468593], length 364 15:21:28.653439 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1426:1442, ack 1382, win 259, options [nop,nop,TS val 2460468612 ecr 25517816], length 16 15:21:28.692996 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1442, win 246, options [nop,nop,TS val 25517828 ecr 2460468612], length 0 15:21:28.693584 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1442:1486, ack 1382, win 259, options [nop,nop,TS val 2460468652 ecr 25517828], length 44 15:21:28.693632 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [.], ack 1486, win 246, options [nop,nop,TS val 25517828 ecr 2460468652], length 0 15:21:28.693706 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1382:1426, ack 1486, win 246, options [nop,nop,TS val 25517828 ecr 2460468652], length 44 15:21:28.694310 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1486:1554, ack 1426, win 259, options [nop,nop,TS val 2460468653 ecr 25517828], length 68 15:21:28.694976 IP 192.168.1.32.ssh > 192.168.1.3.40096: Flags [P.], seq 1426:1478, ack 1554, win 246, options [nop,nop,TS val 25517828 ecr 2460468653], length 52 15:21:28.737617 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [.], ack 1478, win 259, options [nop,nop,TS val 2460468654 ecr 25517828], length 0 15:21:31.599860 IP 192.168.1.3.40096 > 192.168.1.32.ssh: Flags [P.], seq 1554:1702, ack 1478, win 259, options [nop,nop,TS val 2460471559 ecr 25517828], length 148 I have some other ideas that I want to try but right now I focused on getting something working so we can all start hacking. Rao > > I had to bump cb up to 80 bytes in the current mptcp_trunk :/ > > > > > > Christoph > > > > > FYI. I am working on restructuring MPTCP code so it's not > > > intrusive to main > > > TCP code. I will probably try a few other techniques but can use > > > this as > > > well. I will post the changes at the appropriate time. > > Nice, thanks for the update. > > Regards, > Mat

On 10/27/2017 12:57 PM, Christoph Paasch wrote: > > I would love to see the rest of the patch. Especially wrt to writing the > mapping to the header. > > How do you handle segments that are being split while sitting in the > subflow's send-queue and later on need to be retransmitted. Will the > retransmitted segment have the same DSS-mapping as the original > transmission? > > > Thanks, > Christoph > I waited to make sure that my change works for net-next and I have just been able to ssh into and out of a system running netdev + MPTCP using the same layout. Following are the relevant changes. The main observation is that we have all the information to build the DSS header, except for the data-sequence. Which is saved and passed on and requires only a 32bit field. Let me know if anyone sees any issues.

Shoaib I have already provided the SKB changing, I am also using the dev field in skb for the other values. Note the above mapping is based on a dated version of the code, It does not work for net-next for which I am using different mapping, but the idea is the same and it does not require any additional information. Changes to mptcp_skb_entail() rshoaib@caduceus5:~/independent_mptcp$ diff /tmp/modified /tmp/original 10c10 <         TCP_SKB_CB(skb)->mptcp_flags |= (mpcb->snd_hiseq_index ? --- >         skb->mptcp_flags |= (mpcb->snd_hiseq_index ? 23c23 <     TCP_SKB_CB(skb)->path_mask |= mptcp_pi_to_flag(tp->mptcp->path_index); --- >     TCP_SKB_CB(skb)->mptcp_path_mask |= mptcp_pi_to_flag(tp->mptcp->path_index); 39c39 <         tcb->mptcp_flags |= MPTCPHDR_INF; --- >         skb->mptcp_flags |= MPTCPHDR_INF; 45c45,46 <     mptcp_save_dss_data_seq(tp, subskb); --- >     subskb->mptcp_flags |= MPTCPHDR_SEQ; >     tcb->mptcp_data_seq = tcb->seq; Changes to mptcp_write_dss_mapping which is now called when the options are written. rshoaib@caduceus5:~/independent_mptcp$ diff /tmp/modified /tmp/original 1,2c1,2 < static int mptcp_write_dss_mapping(const struct tcp_sock *tp, <     const struct sk_buff *skb, __be32 *ptr) --- > static int mptcp_write_dss_mapping(const struct tcp_sock *tp, const struct sk_buff *skb, >                    __be32 *ptr) 4a5 >     __be32 *start = ptr; 7c8 <     *ptr++ = htonl(tcb->mptcp_data_seq); /* data_seq */ --- >     *ptr++ = htonl(tcb->seq); /* data_seq */

13c14 <         *ptr++ = htonl(tcb->seq - tp->mptcp->snt_isn); /* subseq */ --- >         *ptr++ = htonl(tp->write_seq - tp->mptcp->snt_isn); /* subseq */

15c16 <     if (skb->mptcp_flags & MPTCPHDR_INF) --- >     if (tcb->mptcp_flags & MPTCPHDR_INF) 17c18 <     else { --- >     else 19,22d19 <         /* mptcp_entail_skb adds one for FIN */ <         if (tcb->tcp_flags & TCPHDR_FIN) <             data_len -= 1; <     } 41c38 <     return mptcp_dss_len/sizeof(*ptr); --- >     return ptr - start; 44,45c41,42 < static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, <     const struct sk_buff *skb, __be32 *ptr) --- > static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, const struct sk_buff *skb, >                     __be32 *ptr) 62d58 <     /* data_ack */ And mptcp_options_write() is now:         if (OPTION_DATA_ACK & opts->mptcp_options) {                 ptr += mptcp_write_dss_data_ack(tp, skb, ptr);                 if (mptcp_is_data_seq(skb)) {                         ptr += mptcp_write_dss_mapping(tp, skb, ptr);                 }                 skb->dev = NULL;         }

On 11/06/2017 02:24 PM, Christoph Paasch wrote: > Hello Rao, > > On 05/11/17 - 18:45:35, Rao Shoaib wrote: >> On 10/27/2017 12:57 PM, Christoph Paasch wrote: >>> I would love to see the rest of the patch. Especially wrt to >>> writing the >>> mapping to the header. >>> >>> How do you handle segments that are being split while sitting in the >>> subflow's send-queue and later on need to be retransmitted. Will the >>> retransmitted segment have the same DSS-mapping as the original >>> transmission? >>> >>> >>> Thanks, >>> Christoph >>> >> I waited to make sure that my change works for net-next and I have >> just been >> able to ssh into and out of a system running netdev + MPTCP using >> the same >> layout. Following are the relevant changes. The main observation is >> that we >> have all the information to build the DSS header, except for the >> data-sequence. Which is saved and passed on and requires only a >> 32bit field. >> >> Let me know if anyone sees any issues. > thanks for sharing, please find inline: > >> Shoaib >> >> I have already provided the SKB changing, I am also using the dev >> field in >> skb for the other values. >> Note the above mapping is based on a dated version of the code, It >> does not >> work for net-next for which I am using different mapping, but the >> idea is >> the same and it does not require any additional information. >> >> Changes to mptcp_skb_entail() >> >> rshoaib@caduceus5:~/independent_mptcp$ diff /tmp/modified /tmp/original >> 10c10 >> <         TCP_SKB_CB(skb)->mptcp_flags |= (mpcb->snd_hiseq_index ? >> --- >>>          skb->mptcp_flags |= (mpcb->snd_hiseq_index ? >> 23c23 >> <     TCP_SKB_CB(skb)->path_mask |= >> mptcp_pi_to_flag(tp->mptcp->path_index); >> --- >>> TCP_SKB_CB(skb)->mptcp_path_mask |= >> mptcp_pi_to_flag(tp->mptcp->path_index); >> 39c39 >> <         tcb->mptcp_flags |= MPTCPHDR_INF; >> --- >>>          skb->mptcp_flags |= MPTCPHDR_INF; >> 45c45,46 >> <     mptcp_save_dss_data_seq(tp, subskb); >> --- >>>      subskb->mptcp_flags |= MPTCPHDR_SEQ; >>>      tcb->mptcp_data_seq = tcb->seq; >> >> Changes to mptcp_write_dss_mapping which is now called when the >> options are >> written. >> >> rshoaib@caduceus5:~/independent_mptcp$ diff /tmp/modified /tmp/original >> 1,2c1,2 >> < static int mptcp_write_dss_mapping(const struct tcp_sock *tp, >> <     const struct sk_buff *skb, __be32 *ptr) >> --- >>> static int mptcp_write_dss_mapping(const struct tcp_sock *tp, const >>> struct >> sk_buff *skb, >>>                     __be32 *ptr) >> 4a5 >>>      __be32 *start = ptr; >> 7c8 >> <     *ptr++ = htonl(tcb->mptcp_data_seq); /* data_seq */ >> --- >>>      *ptr++ = htonl(tcb->seq); /* data_seq */ > mptcp_write_dss_mapping is now being called from tcp_options_write, > through > mptcp_options_write, right? > > At this point, tcb->seq will be the TCP-subflow's sequence number. > > So, I'm not sure how you are able to get the data-sequence number here. Look at the code it is stashed in skb. > >> 13c14 >> <         *ptr++ = htonl(tcb->seq - tp->mptcp->snt_isn); /* subseq */ >> --- >>>          *ptr++ = htonl(tp->write_seq - tp->mptcp->snt_isn); /* >>> subseq */ > this here is what I was worried about. When we are retransmitting a > segment, > tp->write_seq won't match anymore with the segment's sequence number. > > You would need to pick tcb->seq here. Nope everything will just work because nothing is different than what is being done now. I guess example is worth a lot. So here is a tcpdump. Host .3 is running net-next while host .32 is running stock MPTCP kernel. I am ssh'ed from .3 to .32 17:45:02.174393 IP 192.168.1.3.57222 > 192.168.1.32.ssh: Flags [.], ack 8534, win 290, options [nop,nop,TS val 2104717628 ecr 5246410,mptcp dss ack 846209813], length 0 17:45:02.174420 IP 192.168.1.3.57222 > 192.168.1.32.ssh: Flags [.], ack 8626, win 290, options [nop,nop,TS val 2104717628 ecr 5246410,mptcp dss ack 846209849], length 0 17:45:02.636920 IP 192.168.1.32.ssh > 192.168.1.3.57222: Flags [P.], seq 8626:8718, ack 3670, win 263, options [nop,nop,TS val 5247726 ecr 2104717628,mptcp dss ack 2862106049 seq 846209849 subseq 8626 len 92 csum 0x658], length 92 17:45:02.636982 IP 192.168.1.3.57222 > 192.168.1.32.ssh: Flags [.], ack 8718, win 290, options [nop,nop,TS val 2104718091 ecr 5246410,mptcp dss ack 846209941], length 0 17:45:22.920549 IP 192.168.1.32.netbios-ns > 192.168.1.255.netbios-ns: NBT UDP PACKET(137): QUERY; REQUEST; BROADCAST <Bring Down the Interface on .32 > 17:45:37.953517 IP 192.168.1.3.57222 > 192.168.1.32.ssh: Flags [P.], seq 3670:3706, ack 8718, win 290, options [nop,nop,TS val 2104753408 ecr 5246410,mptcp dss ack 846209941 seq 2862106049 subseq 3670 len 36 csum 0xe2fa], length 36 17:45:38.021963 IP 192.168.1.3.57222 > 192.168.1.32.ssh: Flags [P.], seq 3670:3706, ack 8718, win 290, options [nop,nop,TS val 2104753476 ecr 5246410,mptcp dss ack 846209941 seq 2862106049 subseq 3670 len 36 csum 0xe2fa], length 36 When I bring the interface back the connection continues on. Now let's do the reverse 17:57:33.441698 IP 192.168.1.3.ssh > 192.168.1.32.60024: Flags [P.], seq 4190:4274, ack 2626, win 291, options [nop,nop,TS val 2105468922 ecr 5435422,mptcp dss ack 1290518463 seq 2562035193 subseq 4190 len 84 csum 0x6c5], length 84 17:57:33.441804 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [.], ack 4274, win 271, options [nop,nop,TS val 5435430 ecr 2105468922,mptcp dss ack 2562035277], length 0 17:57:33.576747 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2626:2662, ack 4274, win 271, options [nop,nop,TS val 5435464 ecr 2105468922,mptcp dss ack 2562035277 seq 1290518463 subseq 2626 len 36 csum 0xc5b9], length 36 17:57:33.577426 IP 192.168.1.3.ssh > 192.168.1.32.60024: Flags [P.], seq 4274:4310, ack 2662, win 291, options [nop,nop,TS val 2105469058 ecr 5435464,mptcp dss ack 1290518499 seq 2562035277 subseq 4274 len 36 csum 0x64d], length 36 17:57:33.577603 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [.], ack 4310, win 271, options [nop,nop,TS val 5435464 ecr 2105469058,mptcp dss ack 2562035313], length 0 17:57:33.609786 IP 192.168.1.3.ssh > 192.168.1.32.60024: Flags [P.], seq 4310:4394, ack 2662, win 291, options [nop,nop,TS val 2105469090 ecr 5435464,mptcp dss ack 1290518499 seq 2562035313 subseq 4310 len 84 csum 0x5d5], length 84 17:57:33.609930 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [.], ack 4394, win 271, options [nop,nop,TS val 5435472 ecr 2105469090,mptcp dss ack 2562035397], length 0 Interface on .3 is down 17:58:04.455423 IP 192.168.1.32.59734 > 192.168.1.3.ssh: Flags [.], ack 705070687, win 272, options [nop,nop,TS val 5443184 ecr 3451436041,mptcp dss ack 426484590], length 0 17:58:05.321012 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2662:2698, ack 4394, win 271, options [nop,nop,TS val 5443400 ecr 2105469090,mptcp dss ack 2562035397 seq 1290518499 subseq 2662 len 36 csum 0xd5ad], length 36 17:58:05.513018 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2698:2734, ack 4394, win 271, options [nop,nop,TS val 5443448 ecr 2105469090,mptcp dss ack 2562035397 seq 1290518535 subseq 2698 len 36 csum 0x7da8], length 36 17:58:05.527395 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2698:2734, ack 4394, win 271, options [nop,nop,TS val 5443452 ecr 2105469090,mptcp dss ack 2562035397 seq 1290518535 subseq 2698 len 36 csum 0x7da8], length 36 17:58:05.696935 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2734:2770, ack 4394, win 271, options [nop,nop,TS val 5443494 ecr 2105469090,mptcp dss ack 2562035397 seq 1290518571 subseq 2734 len 36 csum 0xebf2], length 36 interface is up and the tcp session continues 17:58:31.975712 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2662:2698, ack 4394, win 271, options [nop,nop,TS val 5450064 ecr 2105469090,mptcp dss ack 2562035397 seq 1290518499 subseq 2662 len 36 csum 0xd5ad], length 36 17:58:31.976255 IP 192.168.1.3.ssh > 192.168.1.32.60024: Flags [P.], seq 4394:4430, ack 2698, win 291, options [nop,nop,TS val 2105527458 ecr 5450064,mptcp dss ack 1290518535 seq 2562035397 subseq 4394 len 36 csum 0x55d], length 36 17:58:31.976307 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [P.], seq 2698:2734, ack 4394, win 271, options [nop,nop,TS val 5450064 ecr 2105527458,mptcp dss ack 2562035397 seq 1290518535 subseq 2698 len 36 csum 0x7da8], length 36 17:58:31.976352 IP 192.168.1.32.60024 > 192.168.1.3.ssh: Flags [.], ack 4430, win 271, options [nop,nop,TS val 5450064 ecr 2105527458,mptcp dss ack 2562035433], length 0 17:58:32.008769 IP 192.168.1.3.ssh > 192.168.1.32.60024: Flags [P.], seq 4430:4514, ack 2734, win 291, options [nop,nop,TS val 2105527490 ecr 5450064,mptcp dss ack 1290518571 seq 2562035433 subseq 4430 len 84 csum 0x4e5], length 84 > However even then, if the segment that is being retransmitted is due > to a > partial ack (e.g., the original transmission was 100 bytes, and we > received > an ack for only 50 bytes). We will then only retransmit the remaining 50 > bytes and thus the relative sequence number won't be the same anymore > as in > the original transmission. The code will do what happens in the current code. Any [re]transmission goes through mptcp_skb_entail(), there the mapping will be updated. Perhaps I should provide you a patch that you can apply and play with. If there are any corner case issues,  I think they can be resolved in the retransmission code etc without requiring any change to the size of skb. Is providing a patch for the latest and greatest MPTCP good enough ? Shoaib

> > Christoph > >> 15c16 >> <     if (skb->mptcp_flags & MPTCPHDR_INF) >> --- >>>      if (tcb->mptcp_flags & MPTCPHDR_INF) >> 17c18 >> <     else { >> --- >>>      else >> 19,22d19 >> <         /* mptcp_entail_skb adds one for FIN */ >> <         if (tcb->tcp_flags & TCPHDR_FIN) >> <             data_len -= 1; >> <     } >> 41c38 >> <     return mptcp_dss_len/sizeof(*ptr); >> --- >>>      return ptr - start; >> 44,45c41,42 >> < static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, >> <     const struct sk_buff *skb, __be32 *ptr) >> --- >>> static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, const >> struct sk_buff *skb, >>>                      __be32 *ptr) >> 62d58 >> <     /* data_ack */ >> >> And mptcp_options_write() is now: >> >>          if (OPTION_DATA_ACK & opts->mptcp_options) { >>                  ptr += mptcp_write_dss_data_ack(tp, skb, ptr); >>                  if (mptcp_is_data_seq(skb)) { >>                          ptr += mptcp_write_dss_mapping(tp, skb, ptr); >>                  } >>                  skb->dev = NULL; >>          } >> >> _______________________________________________ mptcp mailing list mptcp(a)lists.01.org https://lists.01.org/mailman/listinfo/mptcp

Maybe to clarify, I meant TCP-level retransmissions (e.g., due to 3 duplicate acks). Not MPTCP-level retransmissions that are triggered through the meta-level retransmission timer. TCP-level retransmissions don't go through mptcp_skb_entail().

> Perhaps I should provide you a patch that you can apply and play with. If > there are any corner case issues,  I think they can be resolved in the > retransmission code etc without requiring any change to the size of skb. Is > providing a patch for the latest and greatest MPTCP good enough ? Yes, patch would be great! Based on either mptcp_trunk or mptcp_v0.93. I actually, would love to see mptcp_trunk no more bump up sk_buff->cb to 80 bytes. So, you can post it also on the mptcp-dev mailing-list if you think it is all fine. Make sure to test it with packet-loss, because that's where I feel is the culprit here. Christoph > Shoaib > >> Christoph >> >>> 15c16 >>> <     if (skb->mptcp_flags & MPTCPHDR_INF) >>> --- >>>>     if (tcb->mptcp_flags & MPTCPHDR_INF) >>> 17c18 >>> <     else { >>> --- >>>>     else >>> 19,22d19 >>> <         /* mptcp_entail_skb adds one for FIN */ >>> <         if (tcb->tcp_flags & TCPHDR_FIN) >>> <             data_len -= 1; >>> <     } >>> 41c38 >>> <     return mptcp_dss_len/sizeof(*ptr); >>> --- >>>>     return ptr - start; >>> 44,45c41,42 >>> < static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, >>> <     const struct sk_buff *skb, __be32 *ptr) >>> --- >>>> static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, const >>> struct sk_buff *skb, >>>>                     __be32 *ptr) >>> 62d58 >>> <     /* data_ack */ >>> >>> And mptcp_options_write() is now: >>> >>>         if (OPTION_DATA_ACK & opts->mptcp_options) { >>>                 ptr += mptcp_write_dss_data_ack(tp, skb, ptr); >>>                 if (mptcp_is_data_seq(skb)) { >>>                         ptr += mptcp_write_dss_mapping(tp, skb, ptr); >>>                 } >>>                 skb->dev = NULL; >>>         } >>> >>>

Here we go. Host .32 running stock MPTCP. I issued following command to randomly drop 30% of incoming packets sudo iptables -A INPUT -m statistic --mode random --probability 0.3 -j DROP Host .3 is running net-next with my changes (There are a lots of changes beside the DSS mapping change)

* First two packets are retransmission as the Receiver dropped the packets 09:25:35.861774 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 3994:4066, ack 40726, win 1029, options [nop,nop,TS val 3951313247 ecr 19355434,mptcp dss ack 3605941855 seq 3439198343 subseq 3994 len 72 csum 0x5107], length 72 09:25:35.935765 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 3994:4066, ack 40726, win 1029, options [nop,nop,TS val 3951313321 ecr 19355434,mptcp dss ack 3605941891 seq 3439198343 subseq 3994 len 72 csum 0x5107], length 72 09:25:35.940562 IP 192.168.1.32.ssh > 192.168.1.3.45056: Flags [P.], seq 40726:40762, ack 4066, win 272, options [nop,nop,TS val 19356053 ecr 3951313321,mptcp dss ack 3439198415 seq 3605941891 subseq 40726 len 36 csum 0x2a7d], length 36 * First 3 packets go through normally. Than we see re-transmissions. 09:27:18.919496 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4422:4458, ack 41262, win 1029, options [nop,nop,TS val 3951416306 ecr 19381477,mptcp dss ack 3605942427 seq 3439198771 subseq 4422 len 36 csum 0x4dd3], length 36 09:27:18.920295 IP 192.168.1.32.ssh > 192.168.1.3.45056: Flags [P.], seq 41262:41298, ack 4458, win 272, options [nop,nop,TS val 19381799 ecr 3951416306,mptcp dss ack 3439198807 seq 3605942427 subseq 41262 len 36 csum 0x48fa], length 36 09:27:18.942893 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [.], ack 41298, win 1029, options [nop,nop,TS val 3951416330 ecr 19381477,mptcp dss ack 3605942463], length 0 09:27:18.967541 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4458:4494, ack 41298, win 1029, options [nop,nop,TS val 3951416354 ecr 19381477,mptcp dss ack 3605942463 seq 3439198807 subseq 4458 len 36 csum 0x4d8b], length 36 09:27:19.047757 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4458:4494, ack 41298, win 1029, options [nop,nop,TS val 3951416435 ecr 19381477,mptcp dss ack 3605942463 seq 3439198807 subseq 4458 len 36 csum 0x4d8b], length 36 09:27:19.284083 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4458:4494, ack 41298, win 1029, options [nop,nop,TS val 3951416671 ecr 19381477,mptcp dss ack 3605942463 seq 3439198807 subseq 4458 len 36 csum 0x4d8b], length 36 09:27:19.284852 IP 192.168.1.32.ssh > 192.168.1.3.45056: Flags [P.], seq 41298:41334, ack 4494, win 272, options [nop,nop,TS val 19381890 ecr 3951416671,mptcp dss ack 3439198843 seq 3605942463 subseq 41298 len 36 csum 0x9e7e], length 36 09:27:19.289617 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4494:4530, ack 41334, win 1029, options [nop,nop,TS val 3951416676 ecr 19381477,mptcp dss ack 3605942463 seq 3439198843 subseq 4494 len 36 csum 0x4d43], length 36 09:27:19.363753 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4494:4530, ack 41334, win 1029, options [nop,nop,TS val 3951416751 ecr 19381477,mptcp dss ack 3605942499 seq 3439198843 subseq 4494 len 36 csum 0x4d43], length 36 09:27:19.368469 IP 192.168.1.32.ssh > 192.168.1.3.45056: Flags [P.], seq 41334:41370, ack 4530, win 272, options [nop,nop,TS val 19381910 ecr 3951416751,mptcp dss ack 3439198879 seq 3605942499 subseq 41334 len 36 csum 0xafd7], length 36 09:27:19.411741 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [.], ack 41370, win 1029, options [nop,nop,TS val 3951416755 ecr 19381477,mptcp dss ack 3605942535], length 0 09:27:20.375482 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951417762 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:20.459886 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951417847 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:21.392076 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951418779 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:23.248063 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951420635 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:26.992046 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951424379 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:31.925768 IP 192.168.1.32.netbios-ns > 192.168.1.255.netbios-ns: NBT UDP PACKET(137): QUERY; REQUEST; BROADCAST 09:27:34.416075 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951431803 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 09:27:49.264080 IP 192.168.1.3.45056 > 192.168.1.32.ssh: Flags [P.], seq 4530:4566, ack 41370, win 1029, options [nop,nop,TS val 3951446651 ecr 19381477,mptcp dss ack 3605942535 seq 3439198879 subseq 4530 len 36 csum 0x4cfb], length 36 It took a while to recover from this one but it finally did, unfortunately I had quit out of tcpdump. The only other test left is partial ACK. I will have to hack the kernel to simulate than so it will take time.

Shoaib On 11/07/2017 09:13 AM, Rao Shoaib wrote: > > > On 11/06/2017 08:09 PM, Christoph Paasch wrote: > > > > Maybe to clarify, I meant TCP-level retransmissions (e.g., due to 3 > > duplicate acks). Not MPTCP-level retransmissions that are triggered > > through > > the meta-level retransmission timer. > > > > TCP-level retransmissions don't go through mptcp_skb_entail(). > Actually they do. When TCP timeout occurs tcp_write_timer_handler() is > called > >         case ICSK_TIME_RETRANS: >                 icsk->icsk_pending = 0; >                 tcp_sk(sk)->ops->retransmit_timer(sk); >                 break; > > retransmit_timer() for sub sockets is initialized to > mptcp_sub_retransmit_timer() which will call mptcp_reinject() except in > the fast open case (something that I need to look at). From what I have > seen there is no case where a packet is [re]transmitted without going  > through mptcp_skb_entail() or else dss will not be updated and the > current code will also not work. Anyways, I will try to find some time > and test with some packet loss. > > If we do find any corner cases I prefer fixing them without exploding > the size of skb. > > Shoaib > > > > > Perhaps I should provide you a patch that you can apply and play > > > with. If > > > there are any corner case issues,  I think they can be resolved in the > > > retransmission code etc without requiring any change to the size > > > of skb. Is > > > providing a patch for the latest and greatest MPTCP good enough ? > > Yes, patch would be great! Based on either mptcp_trunk or mptcp_v0.93. > > > > I actually, would love to see mptcp_trunk no more bump up > > sk_buff->cb to 80 > > bytes. So, you can post it also on the mptcp-dev mailing-list if you > > think > > it is all fine. Make sure to test it with packet-loss, because > > that's where > > I feel is the culprit here. > > > > > > Christoph > > > > > Shoaib > > > > > > > Christoph > > > > > > > > > 15c16 > > > > > <     if (skb->mptcp_flags & MPTCPHDR_INF) > > > > > --- > > > > > >       if (tcb->mptcp_flags & MPTCPHDR_INF) > > > > > 17c18 > > > > > <     else { > > > > > --- > > > > > >       else > > > > > 19,22d19 > > > > > <         /* mptcp_entail_skb adds one for FIN */ > > > > > <         if (tcb->tcp_flags & TCPHDR_FIN) > > > > > <             data_len -= 1; > > > > > <     } > > > > > 41c38 > > > > > <     return mptcp_dss_len/sizeof(*ptr); > > > > > --- > > > > > >       return ptr - start; > > > > > 44,45c41,42 > > > > > < static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, > > > > > <     const struct sk_buff *skb, __be32 *ptr) > > > > > --- > > > > > > static int mptcp_write_dss_data_ack(const struct tcp_sock *tp, const > > > > > struct sk_buff *skb, > > > > > >                       __be32 *ptr) > > > > > 62d58 > > > > > <     /* data_ack */ > > > > > > > > > > And mptcp_options_write() is now: > > > > > > > > > >           if (OPTION_DATA_ACK & opts->mptcp_options) { > > > > >                   ptr += mptcp_write_dss_data_ack(tp, skb, ptr); > > > > >                   if (mptcp_is_data_seq(skb)) { > > > > >                           ptr += > > > > > mptcp_write_dss_mapping(tp, skb, ptr); > > > > >                   } > > > > >                   skb->dev = NULL; > > > > >           } > > > > > > > > > > > > _______________________________________________ > mptcp mailing list > mptcp(a)lists.01.org > https://lists.01.org/mailman/listinfo/mptcp _______________________________________________ mptcp mailing list mptcp(a)lists.01.org https://lists.01.org/mailman/listinfo/mptcp

hese are meta-level retransmissions that are being sent on a different subflow and/or on the same subflow but with new TCP sequence numbers and a new DSS-mapping. These indeed end up going through mptcp_skb_entail(). The retransmissions I mean are the TCP-level retransmissions (aka., fast-retransmits, tail-loss-probe, RTO,...). They don't go through mptcp_skb_entail again. I will take a look at the trace in the other mail. Christoph

On 11/07/2017 01:15 PM, Christoph Paasch wrote: > hese are meta-level retransmissions that are being sent on a different > subflow and/or on the same subflow but with new TCP sequence numbers and a > new DSS-mapping. These indeed end up going through mptcp_skb_entail(). > > The retransmissions I mean are the TCP-level retransmissions (aka., > fast-retransmits, tail-loss-probe, RTO,...). They don't go through > mptcp_skb_entail again. > > I will take a look at the trace in the other mail. > > > Christoph RTO will go through this code. Partial ACK and fast-retransmit etc are fine if they are transmitted with the same mapping (or else DSS will fail as well) In fact it is required. On the receiver an adjustment is made for the TCP flow's seq number.  See mptcp_detect_mapping() it requires that the (partial)  skb has the exact same mapping as if it was transmitted as part of the original skb and than look at mptcp_validate_mapping() and mptcp_prepare_skb() that adjust the data sequence number based on the tcp sequence number of the packet. If not than can you explain how the current mechanism works and what happens to the DSS settings in case of retransmission. Where/How does it gets updated because mptcp_options_write() only does a copy with the updated ACK.

From that moment on it won't change anymore and, as you know, it will simply

When I have some more time I will verify by hacking the kernel. Sorry I can not upload the complete kernel as it is a mess right now, I changed a lot of things, that is why it has taken this long to even get ssh working. I will try to get you just the patch for this fix. Shoaib

On 11/07/2017 04:25 PM, Christoph Paasch wrote: > The way it is handled currently is that the mapping is decided during > mptcp_skb_entail(), which writes the mapping to the skb->cb. > > From that moment on it won't change anymore and, as you know, it will simply > get copied from the skb->cb to the TCP-header in mptcp_options_write(). > > > Christoph Yes thanks I get it. And the receiver is coded that way. That is very implementation specific. RFC 6824 Says:    A data sequence mapping does not need to be included in every MPTCP    packet, as long as the subflow sequence space in that packet is    covered by a mapping known at the receiver.  This can be used to    reduce overhead in cases where the mapping is known in advance; one    such case is when there is a single subflow between the hosts,    another is when segments of data are scheduled in larger than packet-    sized chunks. So DSS mapping is not required in every packet.

There could also be a sub mapping for the segment itself that does not violates the original larger mapping or there can be a super mapping which extends the current mapping without violating it. Current code does not handle that, I will update the code.

Thanks for pointing out this case as I had not tested partial ACK. Without partial ACK everything will work. Luckily the fix is very straight forward. Regards, Shoaib

Here's an implementation of the "shared control buffer" we recently discussed on this list. This uses a SKB_SHINFO_EXT flag for __alloc_skb to optionally allocate extra space following skb_shared_info. Regular (non-extended) skbs are not changed at all. The new skbs with extended shared info can be safely handled by existing code, although low-level operations that copy or reallocate skb data and shared info may strip the extended information if they are not aware of it. The second patch allows users of do_tcp_sendpages to provide a skb with an extended control buffer to use for the first packet. In MPTCP, this would allow a DSS mapping for a large chunk of data to be sent in the first packet, with the rest of the mapped data sent in subsequent packets. It also conveniently ensures that the beginning of the provided data is not coalesced in to an existing skb so the DSS mapping and the beginning of the mapped data stay together. I still need to verify how GSO and TSO handle these skbs. When I tested this code by using SKB_ALLOC_SHINFO_EXT with every TCP allocation, the extended information was available when TCP headers are written. Mat Martineau (2): skbuff: Add shared control buffer tcp: Let do_tcp_sendpages accept a pre-allocated initial skb include/linux/skbuff.h | 24 +++++++++++++++++++++- include/net/tcp.h | 2 +- net/core/skbuff.c | 56 ++++++++++++++++++++++++++++++++++++++------------ net/ipv4/tcp.c | 12 ++++++++--- net/tls/tls_main.c | 3 ++- 5 files changed, 78 insertions(+), 19 deletions(-) -- 2.14.2