[MPTCP] Re: [RFC PATCH 6/6] mptcp: add netlink based PM

[MPTCP] Re: [RFC PATCH 6/6] mptcp: add netlink based PM

[Paolo Abeni]

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

On Tue, 2020-02-11 at 19:12 +0100, Paolo Abeni wrote:
> +/*
> + * ATTR types defined for MPTCP
> + */
> +enum mptcp_pm_attrs {
> +	MPTCP_PM_ATTR_UNSPEC,
> +
> +	MPTCP_PM_ATTR_ADDR_LIST,			/* nested list */
> +	MPTCP_PM_ATTR_ADD_ADDR_MAX,			/* u32 */
> +
> +	__MPTCP_PM_ATTR_MAX
> +};
> +
> +#define MPTCP_PM_ATTR_MAX (__MPTCP_PM_ATTR_MAX - 1)
> +
> +enum mptcp_pm_addr_addrs {
> +	MPTCP_PM_ADDR_ATTR_UNSPEC,
> +
> +	MPTCP_PM_ADDR_ATTR_FAMILY,			/* u16 */
> +	MPTCP_PM_ADDR_ATTR_ADDR4,			/* u32 */
> +	MPTCP_PM_ADDR_ATTR_ADDR6,			/* struct in6_addr */
> +
> +	__MPTCP_PM_ADDR_ATTR_MAX
> +};
> +
> +#define MPTCP_PM_ADDR_ATTR_MAX (__MPTCP_PM_ADDR_ATTR_MAX - 1)
> +
> +enum {
> +	MPTCP_CMD_UNSPEC,
> +
> +	MPTCP_CMD_NS_SIGNAL,
> +	MPTCP_CMD_NS_LOCAL,
> +	MPTCP_CMD_NS_ADD_ADDR_MAX,

Starting the self-test implementation, I noticed changing the above ABI
a bit would make things simpler:

instead of MPTCP_CMD_NS_SIGNAL,

	MPTCP_CMD_SIGNAL_ADD // adds a single addr to the 'signal' list

	MPTCP_CMD_SIGNAL_DEL // remote an address from 'signal' list
				by id
	MPTCP_CMD_SIGNAL_GET	// 'doit' get a single address from 
				//	list, by id
				// 'dumpit' dump the whole list. 
 	MPTCP_CMD_SIGNAL_FLUSH	// remove all addresses from 'signal'
				// list

And similar changes for 'MPTCP_CMD_NS_LOCAL'
Also, I'dd an additional addr MPTCP_PM_ADDR_ATTR_ID 

Any feedback welcome!

Thanks,

Paolo

[MPTCP] Re: [RFC PATCH 6/6] mptcp: add netlink based PM

[Paolo Abeni]

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

On Wed, 2020-02-12 at 15:33 +0100, Florian Westphal wrote:
> > +static const struct nla_policy mptcp_pm_addr_policy[MPTCP_PM_ADDR_ATTR_MAX + 1] = {
> > +	[MPTCP_PM_ADDR_ATTR_FAMILY]	= { .type	= NLA_U16,	},
> > +	[MPTCP_PM_ADDR_ATTR_ADDR4]	= { .type	= NLA_U32,	},
> > +	[MPTCP_PM_ADDR_ATTR_ADDR6]	= { .type	= NLA_BINARY,
> 
> 	NLA_EXACT_LEN?
> 
> NLA_BINARY is weird (it allows shorter sizes...).

yup, thanks! I'm diving into the netlink APIs for the first time ;)

Will fix in the next iteration.

Thanks,

Paolo

[MPTCP] Re: [RFC PATCH 6/6] mptcp: add netlink based PM

[Florian Westphal]

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

Paolo Abeni <pabeni(a)redhat.com> wrote:
> Expose to U/S a netlink family to control the PM, setting:
> 
>  - list of local addresses to be signaled.
>  - list of local addresses used to created subflows.
>  - maximum number of add_addr option to react
> 
> When the msk is fully established, the PM netlink attempts to
> create subflow for each addr in 'local' list, waiting for each
> connection to be completed before attempting the next one.
> 
> After exausting the 'local' list, the PM tries to announce the
> 'signal' list via the ADD_ADDR option. Since we currenlty lack
> the ADD_ADDR echo (and related event) only the first addr is sent.

> Idea is to add an additional PM hook for ADD_ADDR echo, to allow
> the PM netlink announcing multiple addresses, in sequence.

Makes sense, thanks!

> +static const struct nla_policy mptcp_pm_addr_policy[MPTCP_PM_ADDR_ATTR_MAX + 1] = {
> +	[MPTCP_PM_ADDR_ATTR_FAMILY]	= { .type	= NLA_U16,	},
> +	[MPTCP_PM_ADDR_ATTR_ADDR4]	= { .type	= NLA_U32,	},
> +	[MPTCP_PM_ADDR_ATTR_ADDR6]	= { .type	= NLA_BINARY,

	NLA_EXACT_LEN?

NLA_BINARY is weird (it allows shorter sizes...).