On Thu, 2019-10-17 at 15:22 +0200, Matthieu Baerts wrote:
On 17/10/2019 11:00, Paolo Abeni wrote:
> +static bool skb_is_fully_mapped(struct sock *ssk, struct
> + struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
> + unsigned skb_consumed;
> + skb_consumed = tcp_sk(ssk)->copied_seq - TCP_SKB_CB(skb)->seq;
> + if (WARN_ON_ONCE(skb_consumed >= skb->len))
> + return true;
> + return skb->len - skb_consumed <= subflow->map_data_len -
> + mptcp_subflow_get_map_offset(subflow);
Do we need to do something special here and/or above to deal with
By only looking at this chunk of code, it looks like skb_consumed can
have a huge value if one of the two seq numbers have done a wraparound.
Differences are ok even in case of wrap-around. AFAIK, we could have
problem only if skb_consumed would be signed and the distance between
'copied_seq' and 'seq' would grow above 1<<31. So basically we
'seq' can be greated than 'copied_seq' just in case of some bug
elsewhere (in TCP and/or in our calls to tcp_read_sock()), and we
should get a warn on the next line, if so.
Then we might have a warning.
Same below with map_data_len and the offset, no?
mptcp_subflow_get_map_offset() can be greater of map_data_len only if
we hit a bug somewhere else. Other places using that algebra currently
don't warn, so I did not add an explicit check here. I would avoid
that, to avoid adding more warns in subflow.c and protocol.c
For the rest, it looks good to me! I can apply them to avoid problems
with the tests (even if for the moment, the tests are not launched
because the compilation with i386 and without IPV6 fail :-) )