[dpdk-dev] lpm: rte_lpm_iterate() - iterate through the routes

Message ID 1480055491-137021-1-git-send-email-chunguang.yang@windriver.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers

Checks

Context Check Description
checkpatch/checkpatch warning coding style issues

Commit Message

alloc Nov. 25, 2016, 6:31 a.m. UTC
  From: Jörgen Grahn <grahn+src@snipabacken.se>

In practice, there's a need to iterate through the entries
of a rte_lpm, apart from the usual insert/delete/lookup
operations.  This is useful for debugging and perhaps for
things like removing all entries referencing a certain nexthop.

This patch implements this through rte_lpm_iterate(), which
uses a cursor (or iterator) to keep track of the current
position.  Client code doesn't need to be aware of rte_lpm
implementation details.

Change-Id: I28ea3d7d92f318988444553ee2bb30b709bcb3b6
Signed-off-by: Jorgen Grahn <jorgen.grahn@hiq.se>
Signed-off-by: alloc <alloc.young@gmail.com>
---
 lib/librte_lpm/Makefile          |  4 +-
 lib/librte_lpm/rte_lpm_iterate.c | 81 ++++++++++++++++++++++++++++++++++++++++
 lib/librte_lpm/rte_lpm_iterate.h | 56 +++++++++++++++++++++++++++
 3 files changed, 139 insertions(+), 2 deletions(-)
 create mode 100644 lib/librte_lpm/rte_lpm_iterate.c
 create mode 100644 lib/librte_lpm/rte_lpm_iterate.h
  

Comments

Bruce Richardson Feb. 22, 2017, 1:41 p.m. UTC | #1
CC: dev@dpdk.org. Missed that address when pulling from email archive.

> -----Original Message-----
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, February 22, 2017 1:39 PM
> To: chunguang yang <chunguang.yang@windriver.com>
> Subject: Re: [dpdk-dev] [PATCH] lpm: rte_lpm_iterate() - iterate through
> the routes
> 
> On Fri, Nov 25, 2016 at 01:31:31AM -0500, chunguang yang wrote:
> > From: J?rgen Grahn <grahn+src@snipabacken.se>
> >
> > In practice, there's a need to iterate through the entries of a
> > rte_lpm, apart from the usual insert/delete/lookup operations.  This
> > is useful for debugging and perhaps for things like removing all
> > entries referencing a certain nexthop.
> >
> > This patch implements this through rte_lpm_iterate(), which uses a
> > cursor (or iterator) to keep track of the current position.  Client
> > code doesn't need to be aware of rte_lpm implementation details.
> >
> > Change-Id: I28ea3d7d92f318988444553ee2bb30b709bcb3b6
> > Signed-off-by: Jorgen Grahn <jorgen.grahn@hiq.se>
> > Signed-off-by: alloc <alloc.young@gmail.com>
> 
> Apologies for the late review, I missed this patch at the time and only
> spotted it in patchwork now.
> 
> First off, there are a number of checkpatch issues flagged by the
> automated scan. If you still want to continue with this patch for 17.05
> release, you should resubmit with those fixed. Other review comments
> inline below too.
> 
> /Bruce
> 
> > ---
> >  lib/librte_lpm/Makefile          |  4 +-
> >  lib/librte_lpm/rte_lpm_iterate.c | 81
> > ++++++++++++++++++++++++++++++++++++++++
> >  lib/librte_lpm/rte_lpm_iterate.h | 56 +++++++++++++++++++++++++++
> >  3 files changed, 139 insertions(+), 2 deletions(-)  create mode
> > 100644 lib/librte_lpm/rte_lpm_iterate.c  create mode 100644
> > lib/librte_lpm/rte_lpm_iterate.h
> >
> > diff --git a/lib/librte_lpm/Makefile b/lib/librte_lpm/Makefile index
> > 3dc549d..c45da19 100644
> > --- a/lib/librte_lpm/Makefile
> > +++ b/lib/librte_lpm/Makefile
> > @@ -42,10 +42,10 @@ EXPORT_MAP := rte_lpm_version.map  LIBABIVER := 2
> >
> >  # all source are stored in SRCS-y
> > -SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c
> > +SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c
> > +rte_lpm_iterate.c
> 
> I don't see any reason why this needs to be in a new file. Can you
> consider merging it into the existing rte_lpm.c/.h files.
> What about an implementation for IPv6? Any plans for an equivalent
> implementation.
> 
> >
> >  # install this header file
> > -SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include := rte_lpm.h rte_lpm6.h
> > +SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include := rte_lpm.h rte_lpm6.h
> > +rte_lpm_iterate.h
> >
> >  ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
> > SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include += rte_lpm_neon.h diff --git
> > a/lib/librte_lpm/rte_lpm_iterate.c b/lib/librte_lpm/rte_lpm_iterate.c
> > new file mode 100644
> > index 0000000..f643764
> > --- /dev/null
> > +++ b/lib/librte_lpm/rte_lpm_iterate.c
> > @@ -0,0 +1,81 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2014 J?rgen Grahn. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above
> copyright
> > + *       notice, this list of conditions and the following disclaimer
> in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of Intel Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products
> derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > + */
> > +#include "rte_lpm_iterate.h"
> > +#include "rte_lpm.h"
> > +
> > +#include <arpa/inet.h>
> > +
> > +
> > +/**
> > + * Iterate through the lpm, pulling out at most 'buflen' valid routes
> > + * (less means we've hit the end).  The cursor should be initialized
> > + * to { 0, 0 } before the first call.
> > + *
> > + * The routes are partially sorted, by prefix length.  Undefined
> > + * results if the lpm is modified in parallel with or inbetween
> > +calls,
> > + * although the iteration will still terminate properly.
> > + */
> > +unsigned
> > +rte_lpm_iterate(struct rte_lpm_route* const buf, unsigned buflen,
> > +		const struct rte_lpm* lpm,
> > +		struct rte_lpm_cursor* const cursor)
> For the lpm library functions, the lpm parameter is given first. I think
> this should be the same, for consistency.
> 
> > +{
> > +	struct rte_lpm_route* p = buf;
> > +	struct rte_lpm_route* const end = p + buflen;
> > +
> > +	const struct rte_lpm_rule_info* const rinfo = lpm->rule_info;
> > +	const struct rte_lpm_rule* const rtbl = lpm->rules_tbl;
> > +
> > +	unsigned d = cursor->d;
> > +	unsigned n = cursor->n;
> > +
> > +	while(p!=end) {
> > +		if(d==32) break;
> > +		if(n>=rinfo[d].used_rules) {
> > +			d++;
> > +			n = 0;
> > +			continue;
> > +		}
> > +		const struct rte_lpm_rule rule = rtbl[rinfo[d].first_rule +
> n];
> > +		p->addr.s_addr = htonl(rule.ip);
> > +		p->plen = d+1;
> > +		p->nh = rule.next_hop;
> > +		p++;
> > +		n++;
> > +	}
> > +
> > +	cursor->d = d;
> > +	cursor->n = n;
> > +
> > +	return p - buf;
> > +}
> 
> My impression from the description and the function title "iterate" was
> that this would iterate through the lpm table itself, returning all ip
> address and next hop matchings. Instead, it appears that this just returns
> the rules from the rules table.
> 
> Given this, I think the function name and behaviour might be better as
> "rte_lpm_get_rules(lpm, lpm_rules_buffer, num_rules, start_idx)"
> where up to num_rules as filled into lpm_rules_buffer, starting at rule
> start_idx in the list. The return value should indicate the number of
> rules that would be filled into lpm_rules_buffer if it had space. This is
> a standard approach we use for situations like this - if retval <
> num_rules, you have them all, otherwise you need to query again. If you
> want, it's also easy to get all the rules in one go - just make a call
> first with a zero-buffer size, and then use the return value to allocate a
> suitably-sized buffer and call a second time.
> 
> > diff --git a/lib/librte_lpm/rte_lpm_iterate.h
> > b/lib/librte_lpm/rte_lpm_iterate.h
> > new file mode 100644
> > index 0000000..25c7841
> > --- /dev/null
> > +++ b/lib/librte_lpm/rte_lpm_iterate.h
> > @@ -0,0 +1,56 @@
> > +/*-
> > + *   BSD LICENSE
> > + *
> > + *   Copyright(c) 2014 J?rgen Grahn. All rights reserved.
> > + *   All rights reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above
> copyright
> > + *       notice, this list of conditions and the following disclaimer
> in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of Intel Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products
> derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
> COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
> INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
> ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
> TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
> USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
> DAMAGE.
> > + */
> > +#ifndef _RTE_LPM_ITERATE_H_
> > +#define _RTE_LPM_ITERATE_H_
> > +
> > +#include <stdint.h>
> > +#include <netinet/in.h>
> > +
> > +struct rte_lpm;
> > +
> > +struct rte_lpm_cursor {
> > +	unsigned d;
> > +	unsigned n;
> > +};
> 
> While I don't think we need a "cursor" structure - see my proposed API
> above, if we do have one, I think it should be made opaque with an API to
> initialize it.
> 
> > +
> > +struct rte_lpm_route {
> > +	struct in_addr addr;
> > +	uint8_t plen;
> > +	uint8_t nh;
> > +};
> > +
> > +unsigned rte_lpm_iterate(struct rte_lpm_route* buf, unsigned buflen,
> > +			 const struct rte_lpm* lpm,
> > +			 struct rte_lpm_cursor* cursor);
> > +
> > +#endif
> > --
> > 2.7.4
> >
  

Patch

diff --git a/lib/librte_lpm/Makefile b/lib/librte_lpm/Makefile
index 3dc549d..c45da19 100644
--- a/lib/librte_lpm/Makefile
+++ b/lib/librte_lpm/Makefile
@@ -42,10 +42,10 @@  EXPORT_MAP := rte_lpm_version.map
 LIBABIVER := 2
 
 # all source are stored in SRCS-y
-SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c
+SRCS-$(CONFIG_RTE_LIBRTE_LPM) := rte_lpm.c rte_lpm6.c rte_lpm_iterate.c
 
 # install this header file
-SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include := rte_lpm.h rte_lpm6.h
+SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include := rte_lpm.h rte_lpm6.h rte_lpm_iterate.h
 
 ifneq ($(filter y,$(CONFIG_RTE_ARCH_ARM) $(CONFIG_RTE_ARCH_ARM64)),)
 SYMLINK-$(CONFIG_RTE_LIBRTE_LPM)-include += rte_lpm_neon.h
diff --git a/lib/librte_lpm/rte_lpm_iterate.c b/lib/librte_lpm/rte_lpm_iterate.c
new file mode 100644
index 0000000..f643764
--- /dev/null
+++ b/lib/librte_lpm/rte_lpm_iterate.c
@@ -0,0 +1,81 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2014 Jörgen Grahn. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#include "rte_lpm_iterate.h"
+#include "rte_lpm.h"
+
+#include <arpa/inet.h>
+
+
+/**
+ * Iterate through the lpm, pulling out at most 'buflen' valid routes
+ * (less means we've hit the end).  The cursor should be initialized
+ * to { 0, 0 } before the first call.
+ *
+ * The routes are partially sorted, by prefix length.  Undefined
+ * results if the lpm is modified in parallel with or inbetween calls,
+ * although the iteration will still terminate properly.
+ */
+unsigned
+rte_lpm_iterate(struct rte_lpm_route* const buf, unsigned buflen,
+		const struct rte_lpm* lpm,
+		struct rte_lpm_cursor* const cursor)
+{
+	struct rte_lpm_route* p = buf;
+	struct rte_lpm_route* const end = p + buflen;
+
+	const struct rte_lpm_rule_info* const rinfo = lpm->rule_info;
+	const struct rte_lpm_rule* const rtbl = lpm->rules_tbl;
+
+	unsigned d = cursor->d;
+	unsigned n = cursor->n;
+
+	while(p!=end) {
+		if(d==32) break;
+		if(n>=rinfo[d].used_rules) {
+			d++;
+			n = 0;
+			continue;
+		}
+		const struct rte_lpm_rule rule = rtbl[rinfo[d].first_rule + n];
+		p->addr.s_addr = htonl(rule.ip);
+		p->plen = d+1;
+		p->nh = rule.next_hop;
+		p++;
+		n++;
+	}
+
+	cursor->d = d;
+	cursor->n = n;
+
+	return p - buf;
+}
diff --git a/lib/librte_lpm/rte_lpm_iterate.h b/lib/librte_lpm/rte_lpm_iterate.h
new file mode 100644
index 0000000..25c7841
--- /dev/null
+++ b/lib/librte_lpm/rte_lpm_iterate.h
@@ -0,0 +1,56 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2014 Jörgen Grahn. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef _RTE_LPM_ITERATE_H_
+#define _RTE_LPM_ITERATE_H_
+
+#include <stdint.h>
+#include <netinet/in.h>
+
+struct rte_lpm;
+
+struct rte_lpm_cursor {
+	unsigned d;
+	unsigned n;
+};
+
+struct rte_lpm_route {
+	struct in_addr addr;
+	uint8_t plen;
+	uint8_t nh;
+};
+
+unsigned rte_lpm_iterate(struct rte_lpm_route* buf, unsigned buflen,
+			 const struct rte_lpm* lpm,
+			 struct rte_lpm_cursor* cursor);
+
+#endif