| Message ID | 20180401001255.28283-2-blp@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [ovs-dev,1/2] vswitchd: Allow user to directly specify sFlow agent address. | expand |
On Sat, 31 Mar 2018 17:12:55 -0700 Ben Pfaff <blp@ovn.org> wrote: > The --disable-system option indicates that the user wants to avoid > using the host's datapath. This is also a good indication that the > user does not want to use other host resources such as the routing > table, so this commit implements that. > > This fixes a failure in the test "ptap - recirculate after packet_type > change" when the host routing table contained an entry that affected > the generated flow. Without this patch, the commands: > > # ip tuntap add mode tun tun0 > # ip addr add dev tun0 10.8.0.10 > > led to a failure in that test. > > Reported-by: Timothy Redaelli <tredaelli@redhat.com> > Reported-at: > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html > Signed-off-by: Ben Pfaff <blp@ovn.org> --- > lib/ovs-router.c | 19 +++++++++++++++++-- > lib/ovs-router.h | 3 +++ > vswitchd/ovs-vswitchd.c | 2 ++ > 3 files changed, 22 insertions(+), 2 deletions(-) Applied the 2 patches and launched the test-suite (with the openvpn tunnel up) and it passed, including "ptap - recirculate after packet_type change" test. Tested-By: Timothy Redaelli <tredaelli@redhat.com>
On Tue, Apr 03, 2018 at 07:14:25PM +0200, Timothy Redaelli wrote: > On Sat, 31 Mar 2018 17:12:55 -0700 > Ben Pfaff <blp@ovn.org> wrote: > > > The --disable-system option indicates that the user wants to avoid > > using the host's datapath. This is also a good indication that the > > user does not want to use other host resources such as the routing > > table, so this commit implements that. > > > > This fixes a failure in the test "ptap - recirculate after packet_type > > change" when the host routing table contained an entry that affected > > the generated flow. Without this patch, the commands: > > > > # ip tuntap add mode tun tun0 > > # ip addr add dev tun0 10.8.0.10 > > > > led to a failure in that test. > > > > Reported-by: Timothy Redaelli <tredaelli@redhat.com> > > Reported-at: > > https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html > > Signed-off-by: Ben Pfaff <blp@ovn.org> --- > > lib/ovs-router.c | 19 +++++++++++++++++-- > > lib/ovs-router.h | 3 +++ > > vswitchd/ovs-vswitchd.c | 2 ++ > > 3 files changed, 22 insertions(+), 2 deletions(-) > > Applied the 2 patches and launched the test-suite (with the openvpn > tunnel up) and it passed, including "ptap - recirculate after > packet_type change" test. > > Tested-By: Timothy Redaelli <tredaelli@redhat.com> Thanks for testing! I applied this series to master.
On 01.04.2018 03:12, Ben Pfaff wrote: > The --disable-system option indicates that the user wants to avoid using > the host's datapath. This is also a good indication that the user does > not want to use other host resources such as the routing table, so this > commit implements that. > > This fixes a failure in the test "ptap - recirculate after packet_type > change" when the host routing table contained an entry that affected the > generated flow. Without this patch, the commands: > > # ip tuntap add mode tun tun0 > # ip addr add dev tun0 10.8.0.10 > > led to a failure in that test. > > Reported-by: Timothy Redaelli <tredaelli@redhat.com> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html > Signed-off-by: Ben Pfaff <blp@ovn.org> > Tested-By: Timothy Redaelli <tredaelli@redhat.com> > --- > lib/ovs-router.c | 19 +++++++++++++++++-- > lib/ovs-router.h | 3 +++ > vswitchd/ovs-vswitchd.c | 2 ++ > 3 files changed, 22 insertions(+), 2 deletions(-) Hello, Ben. Is it possible to backport these two patches to stable branch-2.9? I have constant unit test failures on my local setup. Best regards, Ilya Maximets.
On Mon, Jun 04, 2018 at 04:49:28PM +0300, Ilya Maximets wrote: > On 01.04.2018 03:12, Ben Pfaff wrote: > > The --disable-system option indicates that the user wants to avoid using > > the host's datapath. This is also a good indication that the user does > > not want to use other host resources such as the routing table, so this > > commit implements that. > > > > This fixes a failure in the test "ptap - recirculate after packet_type > > change" when the host routing table contained an entry that affected the > > generated flow. Without this patch, the commands: > > > > # ip tuntap add mode tun tun0 > > # ip addr add dev tun0 10.8.0.10 > > > > led to a failure in that test. > > > > Reported-by: Timothy Redaelli <tredaelli@redhat.com> > > Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html > > Signed-off-by: Ben Pfaff <blp@ovn.org> > > Tested-By: Timothy Redaelli <tredaelli@redhat.com> > > --- > > lib/ovs-router.c | 19 +++++++++++++++++-- > > lib/ovs-router.h | 3 +++ > > vswitchd/ovs-vswitchd.c | 2 ++ > > 3 files changed, 22 insertions(+), 2 deletions(-) > > Hello, Ben. > Is it possible to backport these two patches to stable branch-2.9? > I have constant unit test failures on my local setup. OK, done.
On 06.06.2018 02:02, Ben Pfaff wrote: > On Mon, Jun 04, 2018 at 04:49:28PM +0300, Ilya Maximets wrote: >> On 01.04.2018 03:12, Ben Pfaff wrote: >>> The --disable-system option indicates that the user wants to avoid using >>> the host's datapath. This is also a good indication that the user does >>> not want to use other host resources such as the routing table, so this >>> commit implements that. >>> >>> This fixes a failure in the test "ptap - recirculate after packet_type >>> change" when the host routing table contained an entry that affected the >>> generated flow. Without this patch, the commands: >>> >>> # ip tuntap add mode tun tun0 >>> # ip addr add dev tun0 10.8.0.10 >>> >>> led to a failure in that test. >>> >>> Reported-by: Timothy Redaelli <tredaelli@redhat.com> >>> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html >>> Signed-off-by: Ben Pfaff <blp@ovn.org> >>> Tested-By: Timothy Redaelli <tredaelli@redhat.com> >>> --- >>> lib/ovs-router.c | 19 +++++++++++++++++-- >>> lib/ovs-router.h | 3 +++ >>> vswitchd/ovs-vswitchd.c | 2 ++ >>> 3 files changed, 22 insertions(+), 2 deletions(-) >> >> Hello, Ben. >> Is it possible to backport these two patches to stable branch-2.9? >> I have constant unit test failures on my local setup. > > OK, done. Thanks.
diff --git a/lib/ovs-router.c b/lib/ovs-router.c index 0f1103b0ed18..ad8bd629e3ab 100644 --- a/lib/ovs-router.c +++ b/lib/ovs-router.c @@ -54,6 +54,10 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER; static struct classifier cls; +/* By default, use the system routing table. For system-independent testing, + * the unit tests disable using the system routing table. */ +static bool use_system_routing_table = true; + struct ovs_router_entry { struct cls_rule cr; char output_bridge[IFNAMSIZ]; @@ -71,13 +75,22 @@ ovs_router_entry_cast(const struct cls_rule *cr) return cr ? CONTAINER_OF(cr, struct ovs_router_entry, cr) : NULL; } +/* Disables obtaining routes from the system routing table, for testing + * purposes. */ +void +ovs_router_disable_system_routing_table(void) +{ + use_system_routing_table = false; +} + static bool ovs_router_lookup_fallback(const struct in6_addr *ip6_dst, char output_bridge[], struct in6_addr *src6, struct in6_addr *gw6) { ovs_be32 src; - if (!route_table_fallback_lookup(ip6_dst, output_bridge, gw6)) { + if (!use_system_routing_table + || !route_table_fallback_lookup(ip6_dst, output_bridge, gw6)) { return false; } if (netdev_get_in4_by_name(output_bridge, (struct in_addr *)&src)) { @@ -238,7 +251,9 @@ void ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen, const char output_bridge[], const struct in6_addr *gw) { - ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw); + if (use_system_routing_table) { + ovs_router_insert__(mark, plen, ip_dst, plen, output_bridge, gw); + } } static void diff --git a/lib/ovs-router.h b/lib/ovs-router.h index b55b1a50b146..f65d652b053d 100644 --- a/lib/ovs-router.h +++ b/lib/ovs-router.h @@ -34,6 +34,9 @@ void ovs_router_insert(uint32_t mark, const struct in6_addr *ip_dst, uint8_t plen, const char output_bridge[], const struct in6_addr *gw); void ovs_router_flush(void); + +void ovs_router_disable_system_routing_table(void); + #ifdef __cplusplus } #endif diff --git a/vswitchd/ovs-vswitchd.c b/vswitchd/ovs-vswitchd.c index 83639357a2ca..414b5478087f 100644 --- a/vswitchd/ovs-vswitchd.c +++ b/vswitchd/ovs-vswitchd.c @@ -38,6 +38,7 @@ #include "openflow/openflow.h" #include "ovsdb-idl.h" #include "ovs-rcu.h" +#include "ovs-router.h" #include "openvswitch/poll-loop.h" #include "simap.h" #include "stream-ssl.h" @@ -219,6 +220,7 @@ parse_options(int argc, char *argv[], char **unixctl_pathp) case OPT_DISABLE_SYSTEM: dp_blacklist_provider("system"); + ovs_router_disable_system_routing_table(); break; case '?':
The --disable-system option indicates that the user wants to avoid using the host's datapath. This is also a good indication that the user does not want to use other host resources such as the routing table, so this commit implements that. This fixes a failure in the test "ptap - recirculate after packet_type change" when the host routing table contained an entry that affected the generated flow. Without this patch, the commands: # ip tuntap add mode tun tun0 # ip addr add dev tun0 10.8.0.10 led to a failure in that test. Reported-by: Timothy Redaelli <tredaelli@redhat.com> Reported-at: https://mail.openvswitch.org/pipermail/ovs-discuss/2018-March/046406.html Signed-off-by: Ben Pfaff <blp@ovn.org> --- lib/ovs-router.c | 19 +++++++++++++++++-- lib/ovs-router.h | 3 +++ vswitchd/ovs-vswitchd.c | 2 ++ 3 files changed, 22 insertions(+), 2 deletions(-)