| Message ID | 892be191-c948-4538-e46d-437c3f3a118c@paneda.se (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | David Ahern |
| Headers | show |
| Series | [iproute2-next,v2] iplink:macvlan: Added bcqueuelen parameter | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
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@paneda.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 > >
On 2020-12-14 18:03, David Ahern wrote: > 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@paneda.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? > > No, 64k is not sufficient when you have very high packet rate and very small packages. I can easily see myself retrieving more than 300 kpps and 65k is then only just around 200 ms of buffer head-room, which I don't consider much saftey margin at all, especially if the incoming data is not perfectly spaced out but rather comes in bursts. If you start adding 10Gbps cards in the mix you could get 10 times that and the buffer would be down to only 20ms. And we would get back to the situation where unicast works fine but multicast does not. So for sure a larger max than 64k is needed. The reason I didn't add an upper limit beside the u32 was that I didn't want to pick an arbitrary limit that works for me now but maybe not support someone elses use case later. I'm now looking at net.core.netdev_max_backlog for inspiration. Which, at least on my system, seems to have a limit of 2147483647 (signed 32 bit int). So perhaps this setting could be aligned with that number since the settings are sort of related instead of using the full range if you prefer that?
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@paneda.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(-) > applied to iproute2-next.
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", 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
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@paneda.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(-)