Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Commit

Permalink
mptcp: Remove cnt_subflows
Browse files Browse the repository at this point in the history
Our goal is to make subflow establishment lockless. cnt_subflows would
need to either become an atomic variable, or we can remove it.

Well, let's remove it! In the general case it is not needed at all. Some
exotic schedulers need cnt_subflows, but we don't care if we add
complexity in these schedulers.

Something special in mptcp_verif_dss_csum(). There the access was actually
wrong. Simply because cnt_subflows > 1, it doesn't mean we have a
working subflow. So, we fix that. I don't think this issue is big enough
to backport it.

Finally mptcp_sched.c was also using cnt_subflows but merely for
speeding up things to avoid going through the whole scheduler code when
there is only one subflow. So, we can remove it and trade performance
for a better design.

Signed-off-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
  • Loading branch information
cpaasch authored and matttbe committed May 2, 2018
1 parent e426daa commit b079a18
Show file tree
Hide file tree
Showing 7 changed files with 30 additions and 25 deletions.
12 changes: 11 additions & 1 deletion include/net/mptcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,6 @@ struct mptcp_cb {
rcv_hiseq_index:1; /* Index in rcv_high_order of rcv_nxt */

/* socket count in this connection */
u8 cnt_subflows;
u8 cnt_established;

#define MPTCP_SCHED_DATA_SIZE 8
Expand Down Expand Up @@ -1294,6 +1293,17 @@ static inline bool mptcp_can_new_subflow(const struct sock *meta_sk)
!tcp_sk(meta_sk)->mpcb->send_infinite_mapping;
}

static inline int mptcp_subflow_count(const struct mptcp_cb *mpcb)
{
struct sock *sk;
int i = 0;

mptcp_for_each_sk(mpcb, sk)
i++;

return i;
}

/* TCP and MPTCP mpc flag-depending functions */
u16 mptcp_select_window(struct sock *sk);
void mptcp_tcp_set_rto(struct sock *sk);
Expand Down
2 changes: 1 addition & 1 deletion net/mptcp/mptcp_binder.c
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ static void create_subflow_worker(struct work_struct *work)
goto exit;

if (mptcp_binder_ndiffports > iter &&
mptcp_binder_ndiffports > mpcb->cnt_subflows) {
mptcp_binder_ndiffports > mptcp_subflow_count(mpcb)) {
struct mptcp_loc4 loc;
struct mptcp_rem4 rem;

Expand Down
15 changes: 6 additions & 9 deletions net/mptcp/mptcp_ctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1379,7 +1379,6 @@ int mptcp_add_sock(struct sock *meta_sk, struct sock *sk, u8 loc_id, u8 rem_id,
mpcb->connection_list = tp;
tp->mptcp->attached = 1;

mpcb->cnt_subflows++;
atomic_add(atomic_read(&((struct sock *)tp)->sk_rmem_alloc),
&meta_sk->sk_rmem_alloc);

Expand All @@ -1399,23 +1398,21 @@ int mptcp_add_sock(struct sock *meta_sk, struct sock *sk, u8 loc_id, u8 rem_id,
sk->sk_destruct = mptcp_sock_destruct;

if (sk->sk_family == AF_INET)
mptcp_debug("%s: token %#x pi %d, src_addr:%pI4:%d dst_addr:%pI4:%d, cnt_subflows now %d\n",
mptcp_debug("%s: token %#x pi %d, src_addr:%pI4:%d dst_addr:%pI4:%d\n",
__func__ , mpcb->mptcp_loc_token,
tp->mptcp->path_index,
&((struct inet_sock *)tp)->inet_saddr,
ntohs(((struct inet_sock *)tp)->inet_sport),
&((struct inet_sock *)tp)->inet_daddr,
ntohs(((struct inet_sock *)tp)->inet_dport),
mpcb->cnt_subflows);
ntohs(((struct inet_sock *)tp)->inet_dport));
#if IS_ENABLED(CONFIG_IPV6)
else
mptcp_debug("%s: token %#x pi %d, src_addr:%pI6:%d dst_addr:%pI6:%d, cnt_subflows now %d\n",
mptcp_debug("%s: token %#x pi %d, src_addr:%pI6:%d dst_addr:%pI6:%d\n",
__func__ , mpcb->mptcp_loc_token,
tp->mptcp->path_index, &inet6_sk(sk)->saddr,
ntohs(((struct inet_sock *)tp)->inet_sport),
&sk->sk_v6_daddr,
ntohs(((struct inet_sock *)tp)->inet_dport),
mpcb->cnt_subflows);
ntohs(((struct inet_sock *)tp)->inet_dport));
#endif

return 0;
Expand Down Expand Up @@ -1452,7 +1449,6 @@ void mptcp_del_sock(struct sock *sk)
}
}
}
mpcb->cnt_subflows--;
if (tp->mptcp->establish_increased)
mpcb->cnt_established--;

Expand Down Expand Up @@ -2816,8 +2812,9 @@ static int mptcp_pm_seq_show(struct seq_file *seq, void *v)
ntohs(isk->inet_dport));
#endif
}

seq_printf(seq, " %02X %02X %08X:%08X %lu",
meta_sk->sk_state, mpcb->cnt_subflows,
meta_sk->sk_state, mptcp_subflow_count(mpcb),
meta_tp->write_seq - meta_tp->snd_una,
max_t(int, meta_tp->rcv_nxt -
meta_tp->copied_seq, 0),
Expand Down
11 changes: 10 additions & 1 deletion net/mptcp/mptcp_input.c
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ static int mptcp_verif_dss_csum(struct sock *sk)

/* Now, checksum must be 0 */
if (unlikely(csum_fold(csum_tcp))) {
struct sock *sk_it = NULL;

pr_err("%s csum is wrong: %#x tcp-seq %u dss_csum_added %d overflowed %d iterations %d\n",
__func__, csum_fold(csum_tcp), TCP_SKB_CB(last)->seq,
dss_csum_added, overflowed, iter);
Expand All @@ -359,7 +361,14 @@ static int mptcp_verif_dss_csum(struct sock *sk)
*/
tp->mpcb->csum_cutoff_seq = tp->mptcp->map_data_seq;

if (tp->mpcb->cnt_subflows > 1) {
/* Search for another subflow that is fully established */
mptcp_for_each_sk(tp->mpcb, sk_it) {
if (sk_it != sk &&
tcp_sk(sk_it)->mptcp->fully_established)
break;
}

if (sk_it) {
mptcp_send_reset(sk);
ans = -1;
} else {
Expand Down
2 changes: 1 addition & 1 deletion net/mptcp/mptcp_ndiffports.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static void create_subflow_worker(struct work_struct *work)
!tcp_sk(mpcb->master_sk)->mptcp->fully_established)
goto exit;

if (num_subflows > iter && num_subflows > mpcb->cnt_subflows) {
if (num_subflows > iter && num_subflows > mptcp_subflow_count(mpcb)) {
if (meta_sk->sk_family == AF_INET ||
mptcp_v6_is_v4_mapped(meta_sk)) {
struct mptcp_loc4 loc;
Expand Down
2 changes: 1 addition & 1 deletion net/mptcp/mptcp_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -1358,7 +1358,7 @@ void mptcp_send_active_reset(struct sock *meta_sk, gfp_t priority)
struct mptcp_cb *mpcb = meta_tp->mpcb;
struct sock *sk;

if (!mpcb->cnt_subflows)
if (!mpcb->connection_list)
return;

WARN_ON(meta_tp->send_mp_fclose);
Expand Down
11 changes: 0 additions & 11 deletions net/mptcp/mptcp_sched.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,6 @@ struct sock *get_available_subflow(struct sock *meta_sk, struct sk_buff *skb,
struct sock *sk;
bool force;

/* if there is only one subflow, bypass the scheduling function */
if (mpcb->cnt_subflows == 1) {
sk = (struct sock *)mpcb->connection_list;
if (!mptcp_is_available(sk, skb, zero_wnd_test))
sk = NULL;
return sk;
}

/* Answer data_fin on same subflow!!! */
if (meta_sk->sk_shutdown & RCV_SHUTDOWN &&
skb && mptcp_is_data_fin(skb)) {
Expand Down Expand Up @@ -268,9 +260,6 @@ static struct sk_buff *mptcp_rcv_buf_optimization(struct sock *sk, int penal)
struct sk_buff *skb_head;
struct defsched_priv *dsp = defsched_get_priv(tp);

if (tp->mpcb->cnt_subflows == 1)
return NULL;

meta_sk = mptcp_meta_sk(sk);
skb_head = tcp_rtx_queue_head(meta_sk);

Expand Down

0 comments on commit b079a18

Please sign in to comment.