netdev - Re: [PATCH] ip-nexthop: support flush by id

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] [day] [month] [year] [list]

Date:   Thu, 1 Apr 2021 09:44:08 +0800
From:   xuchunmei <xuchunmei@...ux.alibaba.com>
To:     David Ahern <dsahern@...il.com>, Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH] ip-nexthop: support flush by id

Yes, I just refer to the support of protocol, not considering that id is unique. It is too heavy to get all.

> 2021年3月31日 下午11:36，David Ahern <dsahern@...il.com> 写道：
> 
> 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.