On 9/9/22 14:46, Ales Musil wrote:
> On Fri, Sep 9, 2022 at 2:40 PM Numan Siddique <[email protected]> wrote:
> 
>> On Fri, Sep 9, 2022 at 2:11 AM Ales Musil <[email protected]> wrote:
>>>
>>> On Fri, Sep 9, 2022 at 7:43 AM Han Zhou <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Sep 6, 2022 at 6:02 AM Ales Musil <[email protected]> wrote:
>>>>>
>>>>> The current behavior is that load balancers
>>>>> default to tcp protocol if it is not specified.
>>>>> However, this is not propagated to lflows which
>>>>> specify proto only when port is specified as part
>>>>> of the protocol e.g. udp.port == X. Always include
>>>>> the protocol in the lb flow even if the port is not
>>>>> specified.
>>>>>
>>>>> Signed-off-by: Ales Musil <[email protected]>
>>>>
>>>> Hi Ales,
>>>>
>>>
>>> Hi Han,
>>>
>>>
>>>> Sorry that I don't understand the problem. The ovn-nb.xml defines:
>>>>
>>>>     <column name="protocol">
>>>>       <p>
>>>>         Valid protocols are <code>tcp</code>, <code>udp</code>, or
>>>>         <code>sctp</code>.  This column is useful when a port number is
>>>>         provided as part of the <code>vips</code> column.  If this
>> column
>>>> is
>>>>         empty and a port number is provided as part of
>> <code>vips</code>
>>>>         column, OVN assumes the protocol to be <code>tcp</code>.
>>>>       </p>
>>>>     </column>
>>>>
>>>> So it defaults to TCP only if a port number is provided. Your patch
>> seems
>>>> to change this definition and is trying to default to TCP even if a
>> port
>>>> number is NOT provided? Did I misunderstand?
>>>>
>>>
>>> right, but everything is presented as tcp being default even if we don't
>>> specify proto e.g. lb-list. Also ovn-northd.8.xml specifies:
>>>
>>> For all the configured load balancing rules for a switch in
>>> <code>OVN_Northbound</code> database that includes a L4 port
>>> <var>PORT</var> of protocol <var>P</var> and IP address
>>> <var>VIP</var>, a priority-120 flow is added.  For IPv4 <var>VIPs
>>> </var>, the flow matches <code>ct.new &amp;&amp; ip &amp;&amp;
>>> ip4.dst == <var>VIP</var> &amp;&amp; <var>P</var> &amp;&amp;
>>> <var>P</var>.dst == <var>PORT</var></code>.  For IPv6 <var>VIPs</var>,
>>> the flow matches <code>ct.new &amp;&amp; ip &amp;&amp; ip6.dst == <var>
>>> VIP </var>&amp;&amp; <var>P</var> &amp;&amp; <var>P</var>.dst == <var>
>>> PORT</var></code>.
>>>
>>> Suggesting that the protocol should be present which was not the case for
>>> even if the port was specified.
>>>
>>> This patch tries to make it consistent. So that we include the protocol
>>> always in the rule and it default to tcp.
>>
>> I think Han is correct.  AFAIK  we do support L3 load balancers i.e if
>> no protocol specified,  then its an
>> L3 load balancer.  If the documentation is not accurate, we should fix
>> that.
>>
> 
> If that's the case we also need to fix lb-list output because it outputs
> tcp even if the port is not specified.
> This was the conclusion after discussion with Dumitru, let's wait for his
> input.
> 

Fixing the lb-list output and maintaining the current load balancer
behavior sounds reasonable to me.

Thanks,
Dumitru

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

Reply via email to