On 3/31/21 5:53 AM, Ido Schimmel wrote:
>> @@ -124,6 +125,9 @@ static int flush_nexthop(struct nlmsghdr *nlh, void *arg)
>> if (tb[NHA_ID])
>> id = rta_getattr_u32(tb[NHA_ID]);
>>
>> + if (filter.id && filter.id != id)
>> + return 0;
>> +
>> if (id && !delete_nexthop(id))
>> filter.flushed++;
>>
>> @@ -491,7 +495,10 @@ static int ipnh_list_flush(int argc, char **argv, int
>> action)
>> NEXT_ARG();
>> if (get_unsigned(&id, *argv, 0))
>> invarg("invalid id value", *argv);
>> - return ipnh_get_id(id);
>> + if (action == IPNH_FLUSH)
>> + filter.id = id;
>> + else
>> + return ipnh_get_id(id);
>
> I think it's quite weird to ask for a dump of all nexthops only to
> delete a specific one. How about this:
+1
>
> ```
> diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c
> index 0263307c49df..09a3076231aa 100644
> --- a/ip/ipnexthop.c
> +++ b/ip/ipnexthop.c
> @@ -765,8 +765,16 @@ static int ipnh_list_flush(int argc, char **argv, int
> action)
> if (!filter.master)
> invarg("VRF does not exist\n", *argv);
> } else if (!strcmp(*argv, "id")) {
> - NEXT_ARG();
> - return ipnh_get_id(ipnh_parse_id(*argv));
> + /* When 'id' is specified with 'flush' / 'list' we do
> + * not need to perform a dump.
> + */
> + if (action == IPNH_LIST) {
> + NEXT_ARG();
> + return ipnh_get_id(ipnh_parse_id(*argv));
> + } else {
> + return ipnh_modify(RTM_DELNEXTHOP, 0, argc,
> + argv);
> + }
since delete just needs the id, you could refactor ipnh_modify and
create a ipnh_delete_id.
