v14:
 - drop mptcp_pm_remove_addrs helper in patch 1
 - add two flags in patch 4, the address entry'll be removed from
   userspace_pm_local_addr_list only when both flags are set, by doing this,
   it's now independent of the order of the remove_subflows command and
   the remove_addrs command.

v13:
 - move the RM_ADDR command after the destruction of the subflow in
   patch 2 and patch 5.
 - drop mptcp_pm_remove_anno_list_by_saddr in mptcp_nl_cmd_sf_destroy in
   patch 4.
 - update userspace_pm.sh too in patch 5.

v12:
 - address Matt's commits in v11.

v11:
 - #1-#5 part 1, address Matt's comments in v10.
 - #6-#9 part 2, update pm mptcp_info
 - #10-#12 part 3, some cleanups.

v10:
 - fix userspace_pm.sh errors reported by CI.
 - fix the bug in mptcp_pm_remove_addrs in patch 1.
 - drop msk->pm.subflow == 1 in mptcp_userspace_pm_delete_local_addr in
   patch 3.
 - exchange the order of "pm_nl_ctl rem" and "pm_nl_ctl dsf" in patch 2
   and 6.
 - update the commit logs.

v9:
 - address Matt's commets in v8.

v8:
 - address Matt's comments.
 - split into two series, pt 2 will send later.

v7:
 - fix userspace_pm.sh errors reported by CI.
 - only remove addrs in mptcp_nl_cmd_remove().

v6:
 - send a RM ADDR from userspace.

v5:
 - fix a memleak error reported by CI.
 - add more delay for userspace pm tests.

v4:
 - add more patches
 - add selftests

v3:
 - update local_addr_used and add_addr_signaled

v2:
 - hold pm locks

Geliang Tang (5):
  mptcp: only send RM_ADDR in nl_cmd_remove
  selftests: mptcp: update userspace pm addr tests
  mptcp: export remove_anno_list_by_saddr
  mptcp: add address into userspace pm list
  selftests: mptcp: update userspace pm subflow tests

 include/uapi/linux/mptcp.h                    |  2 +
 net/mptcp/pm_netlink.c                        |  8 +-
 net/mptcp/pm_userspace.c                      | 77 ++++++++++++++++++-
 net/mptcp/protocol.h                          |  2 +
 .../testing/selftests/net/mptcp/mptcp_join.sh | 11 ++-
 .../selftests/net/mptcp/userspace_pm.sh       |  3 +
 6 files changed, 94 insertions(+), 9 deletions(-)

-- 
2.35.3

Hi Geliang,

On 22/05/2023 15:11, Geliang Tang wrote:
> v14:
>  - drop mptcp_pm_remove_addrs helper in patch 1
>  - add two flags in patch 4, the address entry'll be removed from
>    userspace_pm_local_addr_list only when both flags are set, by doing this,
>    it's now independent of the order of the remove_subflows command and
>    the remove_addrs command.

Thank you for this v14.

Please see my replies and questions on the individual patches but in short:

- Patch 1/5:
  - Maybe best to keep the helper. You can also modify it to remove just
one address instead of a list of addresses.
  - see patch 4/5: you might want to ignore the returned value of
remove_anno_list_by_saddr()

- Patch 3/5:
  - see patch 4/5: I think you can remove this one

- Patch 4/5:
  - I think it is better to use the version from v12: if the userspace
asks to destroy the subflow, we remove the linked address entry (if not
used by another one). So yes, it means we cannot send a RM_ADDR after
the destruction of the subflow but that sounds OK to me. I don't see why
it is needed to close a subflow (send FIN) and then send a RM_ADDR
  - Or am I missing something?
  - (if really we want to send a RM_ADDR for an ID we don't have, we can
also create a dummy entry just to be able to send the RM_ADDR but
currently, I don't see why the userspace will need to do that).
  - compared to v12, please:
    - do not call mptcp_pm_alloc_anno_list() (+
mptcp_pm_remove_anno_list_by_saddr()) from mptcp_nl_cmd_sf_create()
    - and move the comment above mptcp_userspace_pm_delete_local_addr()

- Patch 5/5:
  - first send a RM_ADDR, then a destroy subflow (or adapt the
verification we do not to expect a remove address)

- Patch 6 (patch 1/7 from your v13 part 2):
  - please add it to this series, it is also for -net

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net