Re: [MPTCP] Re: [MPTCP][PATCH mptcp-next 4/6] mptcp: add the DSS checksum sending

[Paolo Abeni <pabeni@redhat.com>]

On Mon, 2021-03-22 at 16:10 -0700, Mat Martineau wrote:
> On Mon, 22 Mar 2021, Geliang Tang wrote:
> 
> > In mptcp_established_options_dss, use the new function
> > mptcp_generate_dss_csum to generate the DSS checksum value.
> > 
> > In mptcp_write_options, send out DSS with the checksum value.
> > 
> > Signed-off-by: Geliang Tang <geliangtang@gmail.com>
> > ---
> > include/net/mptcp.h  |  1 +
> > net/mptcp/options.c  | 41 ++++++++++++++++++++++++++++++++++++++---
> > net/mptcp/protocol.h |  7 +++++++
> > 3 files changed, 46 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/net/mptcp.h b/include/net/mptcp.h
> > index 36fb6907aa6f..869a6d98a72e 100644
> > --- a/include/net/mptcp.h
> > +++ b/include/net/mptcp.h
> > @@ -32,6 +32,7 @@ struct mptcp_ext {
> > 			frozen:1,
> > 			reset_transient:1;
> > 	u8		reset_reason:4;
> > +	u16		csum;
> > };
> > 
> > #define MPTCP_RM_IDS_MAX	8
> > diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> > index bdced173edff..69eb15ef9385 100644
> > --- a/net/mptcp/options.c
> > +++ b/net/mptcp/options.c
> > @@ -521,6 +521,30 @@ static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow,
> > 	}
> > }
> > 
> > +static u16 mptcp_generate_dss_csum(struct sk_buff *skb)
> > +{
> > +	struct csum_pseudo_header header;
> > +	struct mptcp_ext *mpext;
> > +	__wsum csum;
> > +
> > +	if (!skb)
> > +		return 0;
> > +
> > +	mpext = mptcp_get_ext(skb);
> > +	if (!mpext || !mpext->use_map)
> > +		return 0;
> > +
> > +	header.data_seq = mpext->data_seq;
> > +	header.subflow_seq = mpext->subflow_seq;
> > +	header.data_len = mpext->data_len;
> > +	header.csum = 0;
> > +
> > +	csum = skb_checksum(skb, 0, skb->len, 0);
> > +	csum = csum_partial(&header, sizeof(header), csum);
> > +
> > +	return csum_fold(csum);
> > +}
> > +
> > static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> > 					  bool snd_data_fin_enable,
> > 					  unsigned int *size,
> > @@ -544,8 +568,11 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
> > 
> > 		remaining -= map_size;
> > 		dss_size = map_size;
> > -		if (mpext)
> > +		if (mpext) {
> > +			if (msk->csum_reqd)
> > +				mpext->csum = mptcp_generate_dss_csum(skb);
> > 			opts->ext_copy = *mpext;
> > +		}
> > 
> > 		if (skb && snd_data_fin_enable)
> > 			mptcp_write_data_fin(subflow, skb, &opts->ext_copy);
> > @@ -1304,6 +1331,9 @@ void mptcp_write_options(__be32 *ptr, const struct tcp_sock *tp,
> > 			flags |= MPTCP_DSS_HAS_MAP | MPTCP_DSS_DSN64;
> > 			if (mpext->data_fin)
> > 				flags |= MPTCP_DSS_DATA_FIN;
> > +
> > +			if (mpext->csum)
> 
> Instead of relying on a non-zero mpext->csum, what about using 
> opts->csum_enabled instead? (Would have to make sure that flag is set in 
> mptcp_out_options for data packets too)
> 
> This would be more compatible with hardware offload if that gets added 
> later.

Side note, not sure if already discussed in the last mtgs, xmit csum
offload looks doable, togethar with TSO !?! The nic have all the pieces
in place/available to compute it...

/P