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
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
