On Mar 22, 2017, at 1:49 PM, Jarno Rajahalme <ja...@ovn.org> wrote:
>
>
>> On Mar 22, 2017, at 1:43 PM, Ben Pfaff <b...@ovn.org> wrote:
>>
>> On Wed, Mar 22, 2017 at 01:40:10PM -0700, Jarno Rajahalme wrote:
>>>
>>>> On Mar 22, 2017, at 1:21 PM, Ben Pfaff <b...@ovn.org> wrote:
>>>>
>>>> On Wed, Mar 22, 2017 at 09:30:36PM +0530, nusid...@redhat.com wrote:
>>>>> From: Numan Siddique <nusid...@redhat.com>
>>>>>
>>>>> When ovs-vswitchd sends the NX_PACKET_IN2 message, it may not
>>>>> encode ETH_TYPE of the packet. And with the commit daf4d3c18da
>>>>> ("odp: Support conntrack orig tuple key."), the conntrack fields
>>>>> are encoded, if set. After the commit 7befb20d0f70
>>>>> ("nx-match: Fix oxm decode."), ovs-vswitchd is sending OFPBMC_BAD_PREREQ
>>>>> message to the controller for the resumed packets (having conntract
>>>>> fields).
>>>>>
>>>>> With the loose match criteria set to false when
>>>>> ofputil_decode_packet_in_private() is called, the prerequisite check
>>>>> for the mf field "ct_nw_src" is failing as ETH_TYPE is not set.
>>>>
>>>> The original design for NXT_RESUME was that the switch would only be
>>>> decoding a flow that it had itself decoded and therefore any failure to
>>>> decode is a bug. I don't think this design has changed (although I'm
>>>> happy to be corrected) so you may be pointing out a bug that we should
>>>> fix by fixing the flow encoder or decoder.
>>>
>>> OVS still gets back only (metadata) fields that it itself sent. The issue
>>> is that ETH_TYPE is not metadata, hence it is not encoded as part of
>>> metadata. The root of the problem is that I made the conntrack original
>>> direction tuple (which is metadata) to have the packet’s ether type as a
>>> prerequisite. Even if the controller would send down exactly the same
>>> metadata as OVS sent out, the controller may still be allowed to do
>>> whatever with the packet, for example, add MPLS headers, thus changing the
>>> packet to not be an IP packet any more. In this scenario the conntrack
>>> original direction tuple (and conntrack state in general) becomes suspect,
>>> as an MPLS packet is untrackable by conntrack.
>>>
>>> Maybe we should make the NXT_RESUME decoder clear out conntrack metadata if
>>> the packet is not an IP packet (anymore), or if IPv4 metadata is present on
>>> an IPv6 packet or the other way around? To do this we could make the
>>> decoding loose as proposed here, but then explicitly check the packet type
>>> and clear the conntrack metadata if needed. No other metadata has any
>>> prerequisites, and if the controller happened to add metadata that OVS does
>>> not understand it might be OK to ignore those.
>>
>> Do we think that the prerequisite on the conntrack metadata is a
>> valuable one? If it is not, then it would be simple to eliminate the
>> prerequisite.
>
> I’ll look into relaxing that. However, for the kernel datapath this
> prerequisite is essential, and we need to be sure we are not sending down
> flows that match on conntrack original direction tuple for non-IP or
> mismatched IP packets.
>
I just posted a patch:
https://patchwork.ozlabs.org/patch/742385/
> Jarno
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
