[ovs-dev] [PATCH] Add dl_type to flow metadata for correct interpretation of conntrack metadata

Ben Pfaff blp at ovn.org
Wed Oct 25 17:10:31 UTC 2017


On Wed, Oct 25, 2017 at 11:35:52AM +0200, Daniel Alvarez wrote:
> When a packet is sent to the controller, dl_type is not stored in the
> 'ofputil_packet_in_private'. When the packet is resumed, the flow's
> dl_type is 0 and this leads to invalid value in ct_orig_tuple in the
> pkt_metadata.
> 
> This patch adds the dl_type to the metadata so that conntrack
> information can be interpreted correctly when packets are resumed.
> 
> Reported-by: Daniel Alvarez Sanchez <dalvarez at redhat.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-October/339868.html
> Signed-off-by: Daniel Alvarez <dalvarez at redhat.com>
> Signed-off-by: Numan Siddique <nusiddiq at redhat.com>

Thanks a lot for the patch.  I believe it fixes an important bug.

I might add another paragraph to the commit message to really drive home
what's going, perhaps something like this:

    This is a change from the ordinary practice, since flow_get_metadata() is
    only supposed to deal with metadata and dl_type is not metadata.  It is
    necessary when ct_state is involved, though, because ct_state only applies
    in the case of particular Ethertypes (IPv4 and IPv6 currently), so we need
    to add it as a kind of prerequisite.  (This isn't ideal; maybe we didn't
    think through the ct_state mechanism carefully enough.)

However, this breaks several tests (1211 1213 1215 1216 1217 1218 1219
1220 1221 1223 1226).  Would you mind checking that this makes sense for
those tests and updating them as necessary to pass?

Thanks,

Ben.


More information about the dev mailing list