From: David Ahern <dsahern@gmail•com>
To: Ido Schimmel <idosch@idosch•org>, Petr Machata <petrm@nvidia•com>
Cc: Ido Schimmel <idosch@nvidia•com>,
netdev@vger•kernel.org, stephen@networkplumber•org,
razor@blackwall•org
Subject: Re: [PATCH iproute2-next] bridge: Add backup nexthop ID support
Date: Wed, 2 Aug 2023 09:35:12 -0600 [thread overview]
Message-ID: <16c562ce-433d-ac78-95ad-101bef710efc@gmail.com> (raw)
In-Reply-To: <ZMpNRzXKIS7ZzSVN@shredder>
On 8/2/23 6:34 AM, Ido Schimmel wrote:
> On Wed, Aug 02, 2023 at 11:55:26AM +0200, Petr Machata wrote:
>>
>> Ido Schimmel <idosch@nvidia•com> writes:
>>
>>> diff --git a/bridge/link.c b/bridge/link.c
>>> index b35429866f52..c7ee5e760c08 100644
>>> --- a/bridge/link.c
>>> +++ b/bridge/link.c
>>> @@ -186,6 +186,10 @@ static void print_protinfo(FILE *fp, struct rtattr *attr)
>>> ll_index_to_name(ifidx));
>>> }
>>>
>>> + if (prtb[IFLA_BRPORT_BACKUP_NHID])
>>> + print_uint(PRINT_ANY, "backup_nhid", "backup_nhid %u ",
>>> + rta_getattr_u32(prtb[IFLA_BRPORT_BACKUP_NHID]));
>>> +
>>
>> This doesn't build on current main. I think we usually send the relevant
>> header sync patch, but maybe there's an assumption the maintainer pushes
>> it _before_ this patch? I'm not sure, just calling it out.
>
> Not needed. David syncs the headers himself.
just pushed the update.
>
>>
>>> if (prtb[IFLA_BRPORT_ISOLATED])
>>> print_on_off(PRINT_ANY, "isolated", "isolated %s ",
>>> rta_getattr_u8(prtb[IFLA_BRPORT_ISOLATED]));
>>> @@ -311,6 +315,7 @@ static void usage(void)
>>> " [ mab {on | off} ]\n"
>>> " [ hwmode {vepa | veb} ]\n"
>>> " [ backup_port DEVICE ] [ nobackup_port ]\n"
>>> + " [ backup_nhid NHID ]\n"
>>
>> I thought about whether there should be "nobackup_nhid", but no. The
>> corresponding nobackup_port is necessary because it would be awkward to
>> specify "backup_port ''" or something. No such issue with NHID.
>>
>>> " [ self ] [ master ]\n"
>>> " bridge link show [dev DEV]\n");
>>> exit(-1);
>>> @@ -330,6 +335,7 @@ static int brlink_modify(int argc, char **argv)
>>> };
>>> char *d = NULL;
>>> int backup_port_idx = -1;
>>> + __s32 backup_nhid = -1;
>>> __s8 neigh_suppress = -1;
>>> __s8 neigh_vlan_suppress = -1;
>>> __s8 learning = -1;
>>> @@ -493,6 +499,10 @@ static int brlink_modify(int argc, char **argv)
>>> }
>>> } else if (strcmp(*argv, "nobackup_port") == 0) {
>>> backup_port_idx = 0;
>>> + } else if (strcmp(*argv, "backup_nhid") == 0) {
>>> + NEXT_ARG();
>>> + if (get_s32(&backup_nhid, *argv, 0))
>>> + invarg("invalid backup_nhid", *argv);
>>
>> Not sure about that s32. NHID's are unsigned in general. I can add a
>> NHID of 0xffffffff just fine:
>>
>> # ip nexthop add id 0xffffffff via 192.0.2.3 dev Xd
>>
>> (Though ip nexthop show then loops endlessly probably because -1 is used
>> as a sentinel in the dump code. Oops!)
ugh, please send a fix.
next prev parent reply other threads:[~2023-08-02 15:35 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-01 15:21 [PATCH iproute2-next] bridge: Add backup nexthop ID support Ido Schimmel
2023-08-02 9:55 ` Petr Machata
2023-08-02 12:34 ` Ido Schimmel
2023-08-02 13:35 ` Petr Machata
2023-08-02 15:35 ` David Ahern [this message]
2023-08-02 17:22 ` Ido Schimmel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=16c562ce-433d-ac78-95ad-101bef710efc@gmail.com \
--to=dsahern@gmail$(echo .)com \
--cc=idosch@idosch$(echo .)org \
--cc=idosch@nvidia$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=petrm@nvidia$(echo .)com \
--cc=razor@blackwall$(echo .)org \
--cc=stephen@networkplumber$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox