[MPTCP] Re: [PATCH v3 2/4] add support for mptcp netlink interface

[MPTCP] Re: [PATCH v3 2/4] add support for mptcp netlink interface

[Matthieu Baerts]

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

Hi Paolo,

Thank you for the v3!

On 17/04/2020 18:08, Paolo Abeni wrote:
> diff --git a/ip/ipmptcp.c b/ip/ipmptcp.c
> new file mode 100644
> index 00000000..c2122790
> --- /dev/null
> +++ b/ip/ipmptcp.c
> @@ -0,0 +1,436 @@

(...)

> +static int mptcp_parse_limit(int argc, char **argv, struct nlmsghdr *n)
> +{
> +	bool set_rcv_add_addrs = false;
> +	bool set_subflows = false;
> +	__u32 rcv_add_addrs = 0;
> +	__u32 subflows = 0;
> +
> +	while (argc > 0) {
> +		if (matches(*argv, "subflows") == 0) {
> +			NEXT_ARG();
> +
> +			if (get_u32(&subflows, *argv, 0))
> +				invarg("invalid subflows\n", *argv);
> +			set_subflows = true;
> +		} else if (matches(*argv, "add_addr_accepted") == 0) {
> +			NEXT_ARG();
> +
> +			if (get_u32(&rcv_add_addrs, *argv, 0))
> +				invarg("invalid add_addr_accepted\n", *argv);
> +			set_rcv_add_addrs = true;
> +		} else {
> +			invarg("unknown", *argv);

I was going to add a detail here: "\n" seems missing but I just noticed 
it is always added in a few files I opened but not everywhere. 
Apparently, there is a mix usage:

  $ git grep -e "invarg(\".*);$" -- ip/ip*.c | wc -l
  359
  $ git grep -e "invarg(\".*);$" -- ip/ip*.c | grep -c '\\n"'
  161

So I am fine with or without this \n here and above where invarg() is used.


But maybe should we have: "unknown limit"? Up to you!

Of course no need to send a v4 just for that ;-)

Cheers,
Matt
-- 
Matthieu Baerts | R&D Engineer
matthieu.baerts(a)tessares.net
Tessares SA | Hybrid Access Solutions
www.tessares.net
1 Avenue Jean Monnet, 1348 Louvain-la-Neuve, Belgium