[ovs-dev] [PATCH ovn v2] northd: match only on supported protocols to handle_svc_check

[Vladislav Odintsov]

Depending on the udp service, it can reply with some udp data.
In that case ovn-controller will warn with next message:

pinctrl(ovn_pinctrl0)|WARN|handle service check: Unsupported protocol - [11]

This is not something abnormal, so it needs to be fixed.
With this patch ovn-northd changes match of appropriate lflow, which sends
traffic to ovn-controller's pinctrl thread to handle_svc_check action.
Now only supported protocols allowed to reach ovn-controller when destined
to $svc_monitor_mac (tcp, icmp, icmpv6).

Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1913162
Signed-off-by: Vladislav Odintsov 
---
v1 -> v2:
  - Addressed Dumitru's suggestion to match on supported protocols
instead of validating protocol inside pinctrl thread.
---
 northd/northd.c |  2 +-
 tests/ovn-northd.at | 30 +++---
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index a6eca916b..c459887b7 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9124,7 +9124,7 @@ build_lswitch_destination_lookup_bmcast(struct 
ovn_datapath *od,
 ovs_assert(od->nbs);
 
 ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
-  "eth.dst == $svc_monitor_mac",
+  "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
   "handle_svc_check(inport);");
 
 struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw;
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index e3669bdf5..8eadad8bf 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -4974,7 +4974,7 @@ check ovn-nbctl lsp-set-options ls2-ro2 
router-port=ro2-ls2
 ovn-sbctl lflow-list ls1 > ls1_lflows
 AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 's/table=../table=??/' | 
sort], [0], [dnl
   table=??(ls_in_l2_lkup  ), priority=0, match=(1), action=(outport = 
get_fdb(eth.dst); next;)
-  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac), action=(handle_svc_check(inport);)
+  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac && (tcp || icmp || icmp6)), action=(handle_svc_check(inport);)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:01:02), action=(outport = "vm1"; output;)
   table=??(ls_in_l2_lkup  ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
@@ -4986,7 +4986,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
 ovn-sbctl lflow-list ls2 > ls2_lflows
 AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed 's/table=../table=??/' | 
sort], [0], [dnl
   table=??(ls_in_l2_lkup  ), priority=0, match=(1), action=(outport = 
get_fdb(eth.dst); next;)
-  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac), action=(handle_svc_check(inport);)
+  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac && (tcp || icmp || icmp6)), action=(handle_svc_check(inport);)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:02:01), action=(outport = "ls2-ro2"; output;)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:02:02), action=(outport = "vm2"; output;)
   table=??(ls_in_l2_lkup  ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
@@ -5006,7 +5006,7 @@ check ovn-nbctl --wait=sb lr-nat-add ro2 snat 20.0.0.200 
192.168.2.200/30
 ovn-sbctl lflow-list ls1 > ls1_lflows
 AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 's/table=../table=??/' | 
sort], [0], [dnl
   table=??(ls_in_l2_lkup  ), priority=0, match=(1), action=(outport = 
get_fdb(eth.dst); next;)
-  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac), action=(handle_svc_check(inport);)
+  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac && (tcp || icmp || icmp6)), action=(handle_svc_check(inport);)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:01:01), action=(outport = "ls1-ro1"; output;)
   table=??(ls_in_l2_lkup  ), priority=50   , match=(eth.dst == 
00:00:00:00:01:02), action=(outport = "vm1"; output;)
   table=??(ls_in_l2_lkup  ), priority=70   , match=(eth.mcast), 
action=(outport = "_MC_flood"; output;)
@@ -5019,7 +5019,7 @@ AT_CHECK([grep "ls_in_l2_lkup" ls1_lflows | sed 
's/table=../table=??/' | sort],
 ovn-sbctl lflow-list ls2 > ls2_lflows
 AT_CHECK([grep "ls_in_l2_lkup" ls2_lflows | sed 's/table=../table=??/' | 
sort], [0], [dnl
   table=??(ls_in_l2_lkup  ), priority=0, match=(1), action=(outport = 
get_fdb(eth.dst); next;)
-  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst == 
$svc_monitor_mac), action=(handle_svc_check(inport);)
+  table=??(ls_in_l2_lkup  ), priority=110  , match=(eth.dst =

Re: [ovs-dev] [PATCH ovn v2] northd: match only on supported protocols to handle_svc_check

[Dumitru Ceara]

On 5/30/23 14:41, Vladislav Odintsov wrote:
> Depending on the udp service, it can reply with some udp data.
> In that case ovn-controller will warn with next message:
> 
> pinctrl(ovn_pinctrl0)|WARN|handle service check: Unsupported protocol - [11]
> 
> This is not something abnormal, so it needs to be fixed.
> With this patch ovn-northd changes match of appropriate lflow, which sends
> traffic to ovn-controller's pinctrl thread to handle_svc_check action.
> Now only supported protocols allowed to reach ovn-controller when destined
> to $svc_monitor_mac (tcp, icmp, icmpv6).
> 
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1913162
> Signed-off-by: Vladislav Odintsov 
> ---
> v1 -> v2:
>   - Addressed Dumitru's suggestion to match on supported protocols
> instead of validating protocol inside pinctrl thread.
> ---

Applied to main and backported to all stable branches down to 22.03, thanks!

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH ovn v2] northd: match only on supported protocols to handle_svc_check

[Vladislav Odintsov]

Thanks, Dumitru!

> On 8 Jun 2023, at 16:31, Dumitru Ceara  wrote:
> 
> On 5/30/23 14:41, Vladislav Odintsov wrote:
>> Depending on the udp service, it can reply with some udp data.
>> In that case ovn-controller will warn with next message:
>> 
>> pinctrl(ovn_pinctrl0)|WARN|handle service check: Unsupported protocol - [11]
>> 
>> This is not something abnormal, so it needs to be fixed.
>> With this patch ovn-northd changes match of appropriate lflow, which sends
>> traffic to ovn-controller's pinctrl thread to handle_svc_check action.
>> Now only supported protocols allowed to reach ovn-controller when destined
>> to $svc_monitor_mac (tcp, icmp, icmpv6).
>> 
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1913162
>> Signed-off-by: Vladislav Odintsov 
>> ---
>> v1 -> v2:
>>  - Addressed Dumitru's suggestion to match on supported protocols
>>instead of validating protocol inside pinctrl thread.
>> ---
> 
> Applied to main and backported to all stable branches down to 22.03, thanks!
> 
> ___
> dev mailing list
> d...@openvswitch.org 
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Regards,
Vladislav Odintsov

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev