| Message ID | 20230222102952.55787-1-nmiki@yahoo-corp.jp |
|---|---|
| Headers | show |
| Series | Add support for preffered src address in ovs-router | expand |
On 22 Feb 2023, at 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. Hi Nobuhiro, I was on PTO last week, will try to review it later this week, or early next week. //Eelco > 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(-) > > -- > 2.31.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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.
On 2023/03/01 2:37, Ilya Maximets wrote: > 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. Hi, Thanks for your review. The proposed minor changes look good to me. Best Regards, Nobuhiro MIKI
On 2023/03/01 2:37, Ilya Maximets wrote: > 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. Hi, Thanks for your review. The proposed minor changes look good to me. Best Regards, Nobuhiro MIKI
On 28 Feb 2023, at 18:37, Ilya Maximets wrote: > 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. I’m in the process of reviewing, and had similar comments to patch #1. The rest looks good so far. Can you give me till the end of today? I’m wrapping up some testing. //Eelco > Best regards, Ilya Maximets.
On 3/1/23 08:19, Eelco Chaudron wrote: > > > On 28 Feb 2023, at 18:37, Ilya Maximets wrote: > >> 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. > > I’m in the process of reviewing, and had similar comments to patch #1. The rest looks good so far. Can you give me till the end of today? I’m wrapping up some testing. Sure. There is no need to hurry. > > //Eelco > > >> Best regards, Ilya Maximets. >
On 1 Mar 2023, at 11:40, Ilya Maximets wrote: > On 3/1/23 08:19, Eelco Chaudron wrote: >> >> >> On 28 Feb 2023, at 18:37, Ilya Maximets wrote: >> >>> 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. >> >> I’m in the process of reviewing, and had similar comments to patch #1. The rest looks good so far. Can you give me till the end of today? I’m wrapping up some testing. > > Sure. There is no need to hurry. Done, replied to the individual patches. //Eelco
On 3/1/23 11:41, Eelco Chaudron wrote: > > > On 1 Mar 2023, at 11:40, Ilya Maximets wrote: > >> On 3/1/23 08:19, Eelco Chaudron wrote: >>> >>> >>> On 28 Feb 2023, at 18:37, Ilya Maximets wrote: >>> >>>> 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. >>> >>> I’m in the process of reviewing, and had similar comments to patch #1. The rest looks good so far. Can you give me till the end of today? I’m wrapping up some testing. >> >> Sure. There is no need to hurry. > > Done, replied to the individual patches. OK. Good. There seems to be enough little tweaks, that the new version would be easier to work with. Nobuhiro, if you could make an update and send v6, that would be great. Thanks! Best regards, Ilya Maximets.
On 2023/03/01 19:56, Ilya Maximets wrote: > On 3/1/23 11:41, Eelco Chaudron wrote: >> >> >> On 1 Mar 2023, at 11:40, Ilya Maximets wrote: >> >>> On 3/1/23 08:19, Eelco Chaudron wrote: >>>> >>>> >>>> On 28 Feb 2023, at 18:37, Ilya Maximets wrote: >>>> >>>>> 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. >>>> >>>> I’m in the process of reviewing, and had similar comments to patch #1. The rest looks good so far. Can you give me till the end of today? I’m wrapping up some testing. >>> >>> Sure. There is no need to hurry. >> >> Done, replied to the individual patches. > > OK. Good. > > There seems to be enough little tweaks, that the new version would be > easier to work with. > > Nobuhiro, if you could make an update and send v6, that would be great. > > Thanks! Thanks to both of you for your reviews! I will submit new v6 patch set today. Best Regards, Nobuhiro MIKI