lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 14 Dec 2020 10:03:29 -0700
From:   David Ahern <dsahern@...il.com>
To:     Thomas Karlsson <thomas.karlsson@...eda.se>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dsahern@...il.com
Cc:     "davem@...emloft.net" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>, stephen@...workplumber.org,
        kuznet@....inr.ac.ru
Subject: Re: [PATCH iproute2-next v2] iplink:macvlan: Added bcqueuelen
 parameter

On 12/14/20 3:42 AM, Thomas Karlsson wrote:
> This patch allows the user to set and retrieve the
> IFLA_MACVLAN_BC_QUEUE_LEN parameter via the bcqueuelen
> command line argument
> 
> This parameter controls the requested size of the queue for
> broadcast and multicast packages in the macvlan driver.
> 
> If not specified, the driver default (1000) will be used.
> 
> Note: The request is per macvlan but the actually used queue
> length per port is the maximum of any request to any macvlan
> connected to the same port.
> 
> For this reason, the used queue length IFLA_MACVLAN_BC_QUEUE_LEN_USED
> is also retrieved and displayed in order to aid in the understanding
> of the setting. However, it can of course not be directly set.
> 
> Signed-off-by: Thomas Karlsson <thomas.karlsson@...eda.se>
> ---
> 
> Note: This patch controls the parameter added in net-next
> with commit d4bff72c8401e6f56194ecf455db70ebc22929e2
> 
> v2 Rebased on origin/main
> v1 Initial version
> 
>  ip/iplink_macvlan.c   | 33 +++++++++++++++++++++++++++++++--
>  man/man8/ip-link.8.in | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 64 insertions(+), 2 deletions(-)
> 
> diff --git a/ip/iplink_macvlan.c b/ip/iplink_macvlan.c
> index b966a615..302a3748 100644
> --- a/ip/iplink_macvlan.c
> +++ b/ip/iplink_macvlan.c
> @@ -30,12 +30,13 @@
>  static void print_explain(struct link_util *lu, FILE *f)
>  {
>  	fprintf(f,
> -		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS\n"
> +		"Usage: ... %s mode MODE [flag MODE_FLAG] MODE_OPTS [bcqueuelen BC_QUEUE_LEN]\n"
>  		"\n"
>  		"MODE: private | vepa | bridge | passthru | source\n"
>  		"MODE_FLAG: null | nopromisc\n"
>  		"MODE_OPTS: for mode \"source\":\n"
> -		"\tmacaddr { { add | del } <macaddr> | set [ <macaddr> [ <macaddr>  ... ] ] | flush }\n",
> +		"\tmacaddr { { add | del } <macaddr> | set [ <macaddr> [ <macaddr>  ... ] ] | flush }\n"
> +		"BC_QUEUE_LEN: Length of the rx queue for broadcast/multicast: [0-4294967295]\n",

Are we really allowing a BC queue up to 4G? seems a bit much. is a u16
and 64k not more than sufficient?


>  		lu->id
>  	);
>  }
> @@ -62,6 +63,14 @@ static int flag_arg(const char *arg)
>  	return -1;
>  }
>  
> +static int bc_queue_len_arg(const char *arg)
> +{
> +	fprintf(stderr,
> +		"Error: argument of \"bcqueuelen\" must be a positive integer [0-4294967295], not \"%s\"\n",
> +		arg);
> +	return -1;
> +}
> +
>  static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  			  struct nlmsghdr *n)
>  {
> @@ -150,6 +159,14 @@ static int macvlan_parse_opt(struct link_util *lu, int argc, char **argv,
>  		} else if (matches(*argv, "nopromisc") == 0) {
>  			flags |= MACVLAN_FLAG_NOPROMISC;
>  			has_flags = 1;
> +		} else if (matches(*argv, "bcqueuelen") == 0) {
> +			__u32 bc_queue_len;
> +			NEXT_ARG();
> +			
> +			if (get_u32(&bc_queue_len, *argv, 0)) {
> +				return bc_queue_len_arg(*argv);
> +			}
> +			addattr32(n, 1024, IFLA_MACVLAN_BC_QUEUE_LEN, bc_queue_len);
>  		} else if (matches(*argv, "help") == 0) {
>  			explain(lu);
>  			return -1;
> @@ -212,6 +229,18 @@ static void macvlan_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[]
>  	if (flags & MACVLAN_FLAG_NOPROMISC)
>  		print_bool(PRINT_ANY, "nopromisc", "nopromisc ", true);
>  
> +	if (tb[IFLA_MACVLAN_BC_QUEUE_LEN] &&
> +		RTA_PAYLOAD(tb[IFLA_MACVLAN_BC_QUEUE_LEN]) >= sizeof(__u32)) {
> +		__u32 bc_queue_len = rta_getattr_u32(tb[IFLA_MACVLAN_BC_QUEUE_LEN]);
> +		print_luint(PRINT_ANY, "bcqueuelen", "bcqueuelen %lu ", bc_queue_len);
> +	}
> +
> +	if (tb[IFLA_MACVLAN_BC_QUEUE_LEN_USED] &&
> +		RTA_PAYLOAD(tb[IFLA_MACVLAN_BC_QUEUE_LEN_USED]) >= sizeof(__u32)) {
> +		__u32 bc_queue_len = rta_getattr_u32(tb[IFLA_MACVLAN_BC_QUEUE_LEN_USED]);
> +		print_luint(PRINT_ANY, "usedbcqueuelen", "usedbcqueuelen %lu ", bc_queue_len);
> +	}
> +
>  	/* in source mode, there are more options to print */
>  
>  	if (mode != MACVLAN_MODE_SOURCE)
> diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
> index 1ff01744..3516765a 100644
> --- a/man/man8/ip-link.8.in
> +++ b/man/man8/ip-link.8.in
> @@ -1352,6 +1352,7 @@ the following additional arguments are supported:
>  .BR type " { " macvlan " | " macvtap " } "
>  .BR mode " { " private " | " vepa " | " bridge " | " passthru
>  .RB " [ " nopromisc " ] | " source " } "
> +.RB " [ " bcqueuelen " { " LENGTH " } ] "
>  
>  .in +8
>  .sp
> @@ -1395,6 +1396,18 @@ against source mac address from received frames on underlying interface. This
>  allows creating mac based VLAN associations, instead of standard port or tag
>  based. The feature is useful to deploy 802.1x mac based behavior,
>  where drivers of underlying interfaces doesn't allows that.
> +
> +.BR bcqueuelen " { " LENGTH " } "
> +- Set the length of the RX queue used to process broadcast and multicast packets.
> +.BR LENGTH " must be a positive integer in the range [0-4294967295]."
> +Setting a length of 0 will effectively drop all broadcast/multicast traffic.
> +If not specified the macvlan driver default (1000) is used.
> +Note that all macvlans that share the same underlying device are using the same
> +.RB "queue. The parameter here is a " request ", the actual queue length used"
> +will be the maximum length that any macvlan interface has requested.
> +When listing device parameters both the bcqueuelen parameter
> +as well as the actual used bcqueuelen are listed to better help
> +the user understand the setting.
>  .in -8
>  
>  .TP
> @@ -2451,6 +2464,26 @@ Commands:
>  .sp
>  .in -8
>  
> +Update the broadcast/multicast queue length.
> +
> +.B "ip link set type { macvlan | macvap } "
> +[
> +.BI bcqueuelen "  LENGTH  "
> +]
> +
> +.in +8
> +.BI bcqueuelen " LENGTH "
> +- Set the length of the RX queue used to process broadcast and multicast packets.
> +.IR LENGTH " must be a positive integer in the range [0-4294967295]."
> +Setting a length of 0 will effectively drop all broadcast/multicast traffic.
> +If not specified the macvlan driver default (1000) is used.
> +Note that all macvlans that share the same underlying device are using the same
> +.RB "queue. The parameter here is a " request ", the actual queue length used"
> +will be the maximum length that any macvlan interface has requested.
> +When listing device parameters both the bcqueuelen parameter
> +as well as the actual used bcqueuelen are listed to better help
> +the user understand the setting.
> +.in -8
>  
>  .SS  ip link show - display device attributes
>  
> 

Powered by blists - more mailing lists