[MPTCP] Re: [RFC PATCH 03/12] mptcp: trigger msk processing even for OoO data

[MPTCP] Re: [RFC PATCH 03/12] mptcp: trigger msk processing even for OoO data

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 1011 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index beb34b8a5363..e964f3670d75 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -268,6 +268,9 @@ mptcp_subflow_rsk(const struct request_sock *rsk)
>  	return (struct mptcp_subflow_request_sock *)rsk;
>  }
>  
> +#define MPTCP_SUBFLOW_DATA_AVAIL	1
> +#define MPTCP_SUBFLOW_OOO_DATA		2

Nit: maybe make this an enum.  OTOH, at the end of the series,
OOO_DATA seems to be set but never used?
Is this for a future patch  [or maybe i made a mistake applying
the series to my local tree and its used after all...]

>  /* MPTCP subflow context */
>  struct mptcp_subflow_context {
>  	struct	list_head node;/* conn_list of subflows */
> @@ -292,7 +295,7 @@ struct mptcp_subflow_context {
>  		map_valid : 1,
>  		mpc_map : 1,
>  		backup : 1,
> -		data_avail : 1,
> +		data_avail : 2,

... so this is 'enum mptcp_data_avail data_avail'.  Just a suggestion of course.

[MPTCP] Re: [RFC PATCH 03/12] mptcp: trigger msk processing even for OoO data

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 1118 bytes --]

Paolo Abeni <pabeni(a)redhat.com> wrote:
> On Sun, 2020-08-02 at 00:30 +0200, Florian Westphal wrote:
> > Paolo Abeni <pabeni(a)redhat.com> wrote:
> > > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > > index beb34b8a5363..e964f3670d75 100644
> > > --- a/net/mptcp/protocol.h
> > > +++ b/net/mptcp/protocol.h
> > > @@ -268,6 +268,9 @@ mptcp_subflow_rsk(const struct request_sock *rsk)
> > >  	return (struct mptcp_subflow_request_sock *)rsk;
> > >  }
> > >  
> > > +#define MPTCP_SUBFLOW_DATA_AVAIL	1
> > > +#define MPTCP_SUBFLOW_OOO_DATA		2
> > 
> > Nit: maybe make this an enum. 
> 
> And move it outside the bitfield, right? well, looks like the standard
> should allows that, but I would prefer not mixing enum and int into the
> bitfield...

I would not mix it with bitfield either.

> > OTOH, at the end of the series,
> > OOO_DATA seems to be set but never used?
> 
> It' used "indirectly" in mptcp_data_ready(). The latter is called when 
> data_avail != 0, but wakes-up the reader only if
> 
> 	data_avail == MPTCP_SUBFLOW_DATA_AVAIL

Ah, yes, thanks for explaining.

[MPTCP] Re: [RFC PATCH 03/12] mptcp: trigger msk processing even for OoO data

[Paolo Abeni]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On Sun, 2020-08-02 at 00:30 +0200, Florian Westphal wrote:
> Paolo Abeni <pabeni(a)redhat.com> wrote:
> > diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> > index beb34b8a5363..e964f3670d75 100644
> > --- a/net/mptcp/protocol.h
> > +++ b/net/mptcp/protocol.h
> > @@ -268,6 +268,9 @@ mptcp_subflow_rsk(const struct request_sock *rsk)
> >  	return (struct mptcp_subflow_request_sock *)rsk;
> >  }
> >  
> > +#define MPTCP_SUBFLOW_DATA_AVAIL	1
> > +#define MPTCP_SUBFLOW_OOO_DATA		2
> 
> Nit: maybe make this an enum. 

And move it outside the bitfield, right? well, looks like the standard
should allows that, but I would prefer not mixing enum and int into the
bitfield...

> OTOH, at the end of the series,
> OOO_DATA seems to be set but never used?

It' used "indirectly" in mptcp_data_ready(). The latter is called when 
data_avail != 0, but wakes-up the reader only if

	data_avail == MPTCP_SUBFLOW_DATA_AVAIL

otherwise (data_avail == MPTCP_SUBFLOW_OOO_DATA) moves only the data to
the ooo queue.

Thanks!

/P