All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lpm: rte_lpm_iterate() - iterate through the routes
@ 2016-11-25  6:31 chunguang yang
       [not found] ` <20170222133929.GA15184@bricha3-MOBL3.ger.corp.intel.com>
  0 siblings, 1 reply; 2+ messages in thread
From: chunguang yang @ 2016-11-25  6:31 UTC (permalink / raw)
  To: dev; +Cc: Weiwei.Wang, mark.asselstine

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

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
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] lpm: rte_lpm_iterate() - iterate through the routes
       [not found] ` <20170222133929.GA15184@bricha3-MOBL3.ger.corp.intel.com>
@ 2017-02-22 13:41   ` Richardson, Bruce
  0 siblings, 0 replies; 2+ messages in thread
From: Richardson, Bruce @ 2017-02-22 13:41 UTC (permalink / raw)
  To: dev, chunguang yang

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
> >

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-02-22 13:41 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-25  6:31 [PATCH] lpm: rte_lpm_iterate() - iterate through the routes chunguang yang
     [not found] ` <20170222133929.GA15184@bricha3-MOBL3.ger.corp.intel.com>
2017-02-22 13:41   ` Richardson, Bruce

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.