On 2/22/23 11:29, Nobuhiro MIKI wrote:
> With this series, the preferred source address in ovs-router is obtained
> from both ovs/route/add command and kernel FIB.
> 
> v5:
> - Add patch to fix man page
> v4:
> - Add cleanup patch for ovs/route/add
> - Remove unrelated code
> v3:
> - Fix netdev-dummy to support multiple IP addresses
> - Add validation and unit tests for ovs/route/add
> - Refactor parsing for optional parameters in ovs/route/add command
> v2:
> - Add NEWS
> 
> Nobuhiro MIKI (5):
>   netdev-dummy: Support multiple IP addresses.
>   ovs-router: Cleanup parser for ovs/route/add command.
>   ofproto: Fix mam page for tunnel related commands.
>   ovs-router: Introduce src option in ovs/route/add command.
>   route-table: Retrieving the preferred source address from Netlink.
> 
>  NEWS                            |   3 +
>  lib/netdev-dummy.c              |  67 ++++++++++------
>  lib/ovs-router.c                | 137 ++++++++++++++++++++++++--------
>  lib/ovs-router.h                |   3 +-
>  lib/route-table.c               |  16 +++-
>  ofproto/ofproto-tnl-unixctl.man |   9 ++-
>  tests/ovs-router.at             | 105 +++++++++++++++++++++++-
>  tests/system-route.at           |  39 +++++++++
>  8 files changed, 311 insertions(+), 68 deletions(-)
> 

Hi.  Thanks for the patches!

The set looks good to me in general.  But I'd like to propose a
couple of minor changes for the patch #1:

diff --git a/lib/netdev-dummy.c b/lib/netdev-dummy.c
index 7d3d2aa23..7467e9fbc 100644
--- a/lib/netdev-dummy.c
+++ b/lib/netdev-dummy.c
@@ -136,7 +136,7 @@ struct netdev_dummy {
 
     struct pcap_file *tx_pcap, *rxq_pcap OVS_GUARDED;
 
-    struct ovs_list addrs;
+    struct ovs_list addrs OVS_GUARDED;
     struct ovs_list rxes OVS_GUARDED; /* List of child "netdev_rxq_dummy"s. */
 
     struct hmap offloaded_flows OVS_GUARDED;
@@ -814,10 +814,8 @@ netdev_dummy_get_addr_list(const struct netdev *netdev_, 
struct in6_addr **paddr
     struct netdev_addr_dummy *addr_dummy;
 
     ovs_mutex_lock(&netdev->mutex);
-    LIST_FOR_EACH (addr_dummy, node, &netdev->addrs) {
-        cnt++;
-    }
 
+    cnt = ovs_list_size(&netdev->addrs);
     if (!cnt) {
         err = EADDRNOTAVAIL;
         goto out;
@@ -1688,7 +1686,8 @@ pkt_list_delete(struct ovs_list *l)
 }
 
 static void
-addr_list_delete(struct ovs_list *l) {
+addr_list_delete(struct ovs_list *l)
+{
     struct netdev_addr_dummy *addr_dummy;
 
     LIST_FOR_EACH_POP (addr_dummy, node, l) {
---

What do you think?
If that looks good to you, I can fold that in before applying.


Eelco, if you still want to review the set I can wait with applying.
Just let me know.

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to