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