| Message ID | 20210331022234.52977-1-xuchunmei@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | David Ahern |
| Headers | show |
| Series | ip-nexthop: support flush by id | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Not a local patch |
On Wed, Mar 31, 2021 at 10:22:34AM +0800, Chunmei Xu wrote: > add id to struct filter to record the 'id', > filter id only when id is set, otherwise flush all. > > Signed-off-by: Chunmei Xu <xuchunmei@linux.alibaba.com> > --- > ip/ipnexthop.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c > index 22c66491..fd759140 100644 > --- a/ip/ipnexthop.c > +++ b/ip/ipnexthop.c > @@ -21,6 +21,7 @@ static struct { > unsigned int master; > unsigned int proto; > unsigned int fdb; > + unsigned int id; > } filter; > > enum { > @@ -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: ``` 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); + } } else if (!matches(*argv, "protocol")) { __u32 proto; ``` > } else if (!matches(*argv, "protocol")) { > __u32 proto; > > -- > 2.27.0 >
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.
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@gmail.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.
diff --git a/ip/ipnexthop.c b/ip/ipnexthop.c index 22c66491..fd759140 100644 --- a/ip/ipnexthop.c +++ b/ip/ipnexthop.c @@ -21,6 +21,7 @@ static struct { unsigned int master; unsigned int proto; unsigned int fdb; + unsigned int id; } filter; enum { @@ -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); } else if (!matches(*argv, "protocol")) { __u32 proto;
add id to struct filter to record the 'id', filter id only when id is set, otherwise flush all. Signed-off-by: Chunmei Xu <xuchunmei@linux.alibaba.com> --- ip/ipnexthop.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)