Thanks for the notice, I will upload v2 in a few minutes without whitespace 
errors.

Yes, this patch is related to commit e7c9ff0. Both are fixing an issue with 
tunnel port handling, but I think the two issue is logically independent. One 
is with deletion, the other one is with changing dst_port value during 
reconfiguration.
I tried to upload a patch earlier, where I was fixing both issue, then I 
noticed that the first one is fixed in the meantime by zhaozhanxu. I sent a 
note to the list in:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-July/335565.html 
I think both patch is necessary, first one should not be changed. I would be 
happy if it could fit to 2.8, also I think backporting to branch-2.7, 2.6, .. 
would be useful.

Balazs

-----Original Message-----
From: Ben Pfaff [mailto:[email protected]] 
Sent: 02 August 2017 22:48
To: Balazs Nemeth <[email protected]>
Cc: [email protected]
Subject: Re: [ovs-dev] [PATCH] tunnel: Fix deletion of datapath tunnel ports in 
case of reconfiguration

On Mon, Jul 17, 2017 at 12:37:07PM +0000, Balazs Nemeth wrote:
> There is an issue in OVS with tunnel deletion during the
> reconfiguration of OF tunnels. If the dst_port value is changed, the
> old tunnel map entry will not be deleted, because the tp_port
> argument of tnl_port_map_delete() has the new dst_port setting, hence
> the tunnel cannot be found in the list of tnl_port structures.
> 
> The patch corrects this mechanism by adding a new argument,
> 'old_odp_port' to tnl_port_reconfigure(). This value is used to
> identify the datapath tunnel port which is being reconfigured. In
> connection with this fix, to unify the tunnel port map handling,
> odp_port value is used to search the proper port to insert and delete
> tunnel map entries as well. This variable can be used instead of
> tp_port, as it is unique for all datapath tunnel ports, and there is
> no need to reach dst_port from netdev_tunnel_config structure.
> 
> This patch also adds a printout to check the reference counter of
> a tnl_port structure in tnl-port.c. Extending OVS unit test cases to
> have ref_cnt values in the expected dump. Adding new test cases to
> check if packet receiving is still working in the case of OF tunnel
> port deletion. Adding new test cases to check the reference counter
> in case of OF tunnel deletion or reconfiguration.
> 
> Signed-off-by: Balazs Nemeth <[email protected]>
> Signed-off-by: Jan Scheurich <[email protected]>
> Co-authored-by: Jan Scheurich <[email protected]>

Thanks for finding and fixing the problem.  This sounds like it fixes a
serious problem in the tunnel implementation.  I'd like to get it in.

There are a few issues.  First, the patch is whitespace damaged and
cannot be applied.  The first for this is usually to use "git
send-email" to send the patch or, if this is difficult, to post it to
Github as a pull request.

Second, is this related to the following patch?  Does either one mean
that the other is not necessary or should change?

    commit e7c9ff0ed108d73f4aca2d5e54fa1e671e935e20
    Author: zhaozhanxu <[email protected]>
    Date:   Mon Jun 26 18:29:22 2017 +0800

        tnl-ports: Fix loss of tunneling upon removal of a single tunnel port.

        When OVS had multiple tunnel ports of a single kind, and any one of them
        was removed, the remaining ports could no longer receive traffic.  This
        fixes the problem.

        Signed-off-by: zhaozhanxu <[email protected]>
        Signed-off-by: Ben Pfaff <[email protected]>

Finally, we are branching for the 2.8 release soon, but that does not
affect the status of this patch because bug fixes are welcomed at any
stage of development.

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to