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