| 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
| ||
|
Message-ID: <20250715070308.63e8841f@hermes.local>
Date: Tue, 15 Jul 2025 07:03:08 -0700
From: Stephen Hemminger <stephen@...workplumber.org>
To: David Wilder <wilder@...ibm.com>
Cc: netdev@...r.kernel.org, jv@...sburgh.net, pradeeps@...ux.vnet.ibm.com,
pradeep@...ibm.com, i.maximets@....org, amorenoz@...hat.com,
haliu@...hat.com, dsahern@...il.com
Subject: Re: [PATCH iproute2-next v2 1/1] iproute: Extend bonding's
"arp_ip_target" parameter to add vlan tags.
On Mon, 14 Jul 2025 16:05:40 -0700
David Wilder <wilder@...ibm.com> wrote:
> This change extends the "arp_ip_target" parameter format to allow for
> a list of vlan tags to be included for each arp target.
>
> The new format for arp_ip_target is:
> arp_ip_target=ipv4-address[vlan-tag\...],...
>
> Examples:
> arp_ip_target=10.0.0.1[10]
> arp_ip_target=10.0.0.1[100/200]
>
> The inclusion of a list of vlan tags is optional. The new logic
> preserves both forward and backward compatibility with the kernel
> and iproute2 versions.
>
> Signed-off-by: David Wilder <wilder@...ibm.com>
> ---
> ip/iplink_bond.c | 117 +++++++++++++++++++++++++++++++++++++++++++----
> 1 file changed, 108 insertions(+), 9 deletions(-)
>
> diff --git a/ip/iplink_bond.c b/ip/iplink_bond.c
> index 62dd907c..c4db68a7 100644
> --- a/ip/iplink_bond.c
> +++ b/ip/iplink_bond.c
> @@ -173,6 +173,53 @@ static void explain(void)
> print_explain(stderr);
> }
>
> +#define BOND_VLAN_PROTO_NONE htons(0xffff)
> +#define BOND_MAX_VLAN_TAGS 5
> +
> +struct bond_vlan_tag {
> + __be16 vlan_proto;
> + __be16 vlan_id;
> +};
> +
> +static inline struct bond_vlan_tag *bond_vlan_tags_parse(char *vlan_list, int level, int *size)
No good reason to inline
> +{
> + struct bond_vlan_tag *tags = NULL;
> + char *vlan;
> + int n;
> +
> + if (level > BOND_MAX_VLAN_TAGS) {
> + fprintf(stderr,
> + "Error: Too many vlan tags specified, maximum is %d.\n",
> + BOND_MAX_VLAN_TAGS);
> + exit(1);
> + }
> +
> + if (!vlan_list || strlen(vlan_list) == 0) {
> + tags = calloc(level + 1, sizeof(*tags));
> + *size = (level + 1) * (sizeof(*tags));
> + if (tags)
> + tags[level].vlan_proto = BOND_VLAN_PROTO_NONE;
> + return tags;
> + }
> +
> + for (vlan = strsep(&vlan_list, "/"); (vlan != 0); level++) {
> + tags = bond_vlan_tags_parse(vlan_list, level + 1, size);
> + if (!tags)
> + continue;
> +
> + tags[level].vlan_proto = htons(ETH_P_8021Q);
> + n = sscanf(vlan, "%hu", &(tags[level].vlan_id));
> +
> + if (n != 1 || tags[level].vlan_id < 1 ||
> + tags[level].vlan_id > 4094)
> + return NULL;
> +
> + return tags;
> + }
> +
> + return NULL;
> +}
> +
> static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
> struct nlmsghdr *n)
> {
> @@ -239,12 +286,29 @@ static int bond_parse_opt(struct link_util *lu, int argc, char **argv,
> NEXT_ARG();
> char *targets = strdupa(*argv);
> char *target = strtok(targets, ",");
> - int i;
> + struct bond_vlan_tag *tags;
> + int size, i;
>
> for (i = 0; target && i < BOND_MAX_ARP_TARGETS; i++) {
> - __u32 addr = get_addr32(target);
> + struct Data {
> + __u32 addr;
> + struct bond_vlan_tag vlans[];
> + } data;
> + char *vlan_list, *dup;
> +
> + dup = strdupa(target);
> + data.addr = get_addr32(strsep(&dup, "["));
> + vlan_list = strsep(&dup, "]");
> +
> + if (vlan_list) {
> + tags = bond_vlan_tags_parse(vlan_list, 0, &size);
> + memcpy(&data.vlans, tags, size);
> + addattr_l(n, 1024, i, &data,
> + sizeof(data.addr)+size);
> + } else {
> + addattr32(n, 1024, i, data.addr);
> + }
>
> - addattr32(n, 1024, i, addr);
> target = strtok(NULL, ",");
> }
> addattr_nest_end(n, nest);
> @@ -507,12 +571,47 @@ static void bond_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
> print_string(PRINT_FP, NULL, "arp_ip_target ", NULL);
> }
>
> - for (i = 0; i < BOND_MAX_ARP_TARGETS; i++) {
> - if (iptb[i])
> - print_string(PRINT_ANY,
> - NULL,
> - "%s",
> - rt_addr_n2a_rta(AF_INET, iptb[i]));
> + for (int i = 0; i < BOND_MAX_ARP_TARGETS && iptb[i]; i++) {
> + struct Data {
> + __u32 addr;
> + struct bond_vlan_tag vlans[BOND_MAX_VLAN_TAGS + 1];
> + } data;
> +
> + if (RTA_PAYLOAD(iptb[i]) < sizeof(data.addr) ||
> + RTA_PAYLOAD(iptb[i]) > sizeof(data)) {
> + fprintf(stderr, "Internal Error: Bad payload for arp_ip_target.\n");
> + exit(1);
> + }
> + memcpy(&data, RTA_DATA(iptb[i]), RTA_PAYLOAD(iptb[i]));
> +
> + print_string(PRINT_ANY,
> + NULL,
> + "%s",
> + rt_addr_n2a(AF_INET, sizeof(data.addr), &data.addr));
I know you just moved this line, but can you shorten it, and use print_color_string?
print_color_string(PRINT_ANY, COLOR_INET, NULL, "%s",
rt_addr_n2a(AF_INET, sizeof(data.addr), &data_addr));
> +
> + if (RTA_PAYLOAD(iptb[i]) > sizeof(data.addr) && !is_json_context()) {
> + print_string(PRINT_ANY, NULL, "[", NULL);
> +
> + for (int level = 0;
iproute2 follows kernel style and avoids using declaration in for statement
> + (data.vlans[level].vlan_proto != BOND_VLAN_PROTO_NONE);
paren not needed for single conditional.
> + level++) {
> +
> + if (level > BOND_MAX_VLAN_TAGS) {
> + fprintf(stderr,
> + "Internal Error: too many vlan tags.\n");
> + exit(1);
> + }
> +
> + if (level != 0)
> + print_string(PRINT_ANY, NULL, "/", NULL);
> +
> + print_uint(PRINT_ANY,
> + NULL, "%u", data.vlans[level].vlan_id);
> + }
> +
> + print_string(PRINT_ANY, NULL, "]", NULL);
Did you check the JSON output? Looks like it won't print the tags.
> + }
> +
> if (!is_json_context()
> && i < BOND_MAX_ARP_TARGETS-1
> && iptb[i+1])
Powered by blists - more mailing lists