On 5/24/23 13:03, Ales Musil wrote:
> On Mon, May 22, 2023 at 11:18 PM Vladislav Odintsov <odiv...@gmail.com>
> 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]
>>
>> With this patch ovn-controller ignores UDP packets, which came to
>> pinctrl_handle_svc_check().  This is not something abnormal, so don't
>> write warnings.
>>
>> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=1913162
>> Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
>> ---
>>  controller/pinctrl.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index b4be22020..f2e753cdb 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -7777,6 +7777,13 @@ pinctrl_handle_svc_check(struct rconn *swconn,
>> const struct flow *ip_flow,
>>          ip_proto = in_ip->ip6_nxt;
>>      }
>>
>> +    if (ip_proto == IPPROTO_UDP) {
>> +        /* We should do nothing if we got UDP packet.
>> +         * If service is running, it can respond with any UDP data,
>> +         * so just ingore it. */
>> +         return;
>> +    }
>> +
>>      if (ip_proto != IPPROTO_TCP && ip_proto != IPPROTO_ICMP &&
>>          ip_proto != IPPROTO_ICMPV6) {
>>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
>> --
>> 2.36.1
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Looks good to me, thanks.
> 
> Acked-by: Ales Musil <amu...@redhat.com>
> 

Hi, Vladislav, Ales,

I think it might be better to just restrict the logical flow condition
to the type of traffic we actually want to handle in slow path (in
pinctrl).

That is, this flow (and the others that match on ethernet destination
being equal to $svc_monitor_mac):

    ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
                  "eth.dst == $svc_monitor_mac",
                  "handle_svc_check(inport);");

should probably be changed to explicitly match on the supported protocol
list, e.g.:

    ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 110,
                  "eth.dst == $svc_monitor_mac && (tcp || icmp || icmp6)",
                  "handle_svc_check(inport);");

Thanks,
Dumitru

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

Reply via email to