Re: [PATCH mptcp-next 2/6] mptcp: introduce and use mptcp_pm_send_ack()

[Paolo Abeni <pabeni@redhat.com>]

On Fri, 2022-07-01 at 17:13 -0700, Mat Martineau wrote:
> On Fri, 1 Jul 2022, Paolo Abeni wrote:
> 
> > The in-kernel PM has a bit of duplicate code related to ack
> > generation. Create a new helper factoring out the PM-specific
> > needs and use it in a couple of places.
> > 
> > As a bonus, mptcp_subflow_send_ack() is not used anymore
> > outside its own compilation unit and can become static.
> > 
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> 
> The rest of the series looks good, I ran the new tests and checked the 
> pcaps too.
> 
> However, there's a conflict in this patch with the export branch. Can you 
> rebase and repost?

I'm surprised by the conflict. I likely messed-up something in my tree.
I'll rebase.

> Something else I noticed that is out of scope for this series, but 
> something we should think about addressing:
> 
> The RFC says about MP_PRIO that "this signal applies to a single 
> direction, and so the sender of this option could choose to continue using 
> the subflow to send data even if it has signaled B=1 to the other host.".
> 
> However, we only have a single 'backup' value stored per subflow. If 
> there's a netlink request to change the backup bit, it affects both 
> outgoing scheduling and sends MP_PRIO to the peer to affect incoming 
> traffic. A received MP_PRIO would affect outgoing packet scheduling on 
> that subflow, but the peer may choose to schedule differently (and we do 
> handle this ok). Would it be worth it to separate "incoming priority" and 
> "outgoing priority"?

Indeed, that could be worthy. Note that we have a very similar issue
for fallback to TCP: according to the RFC is again an unidirectional
status, but we have a single bit info for both directions.

I guess the relevant changes (both for MP_PRIO and even more for
fallback could be a bit invasive, let's see...

Cheers,

Paolo