Hi guys, Great job Numan! As we discussed over IRC, the patch below may make more sense. It essentially sets the dl_type so that when packet comes from the controller, it matches a valid type and OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 is not added. Maybe what Numan proposed and this patch could be a good combination but I think that we definitely need to set the dl_type as it's later checked in pkt_metadata_from_flow() and it'll be zero there otherwise. What do you guys think? I have tried the patch below and the kernel error is not shown anymore when issuing DHCP requests.
diff --git a/lib/flow.c b/lib/flow.c index b2b10aa..62b948f 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -1073,6 +1073,7 @@ flow_get_metadata(const struct flow *flow, struct match *flow_metadata) if (flow->ct_state != 0) { match_set_ct_state(flow_metadata, flow->ct_state); + match_set_dl_type(flow_metadata, flow->dl_type); if (is_ct_valid(flow, NULL, NULL) && flow->ct_nw_proto != 0) { if (flow->dl_type == htons(ETH_TYPE_IP)) { match_set_ct_nw_src(flow_metadata, flow->ct_nw_src); Thanks, Daniel On Tue, Oct 24, 2017 at 10:38 PM, Ben Pfaff <b...@ovn.org> wrote: > On Tue, Oct 24, 2017 at 09:04:22PM +0530, Numan Siddique wrote: > > We did some more investigation. This issue is seen only when OVN native > > dhcp is used and with kernel datapath which doesn't support > > OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4. The reason for this failure is because > > ovs-vswitchd includes the attribute OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 > when it > > sends the packet back to the datapath after the dhcp reply packet is > > resumed. > > > > When the dhcp packet is sent to ovn-controller, the ct_state value is set > > to 0x21 and dl_type is set to 0 in the flow metadata. When the packet is > > resumed, the function nxt_resume() calls 'pkt_metadata_from_flow()' which > > neither sets 'md->ct_orig_tuple' or memsets it [1] because is_ct_valid() > > returns true and dl_type is 0. And the function odp_key_from_dp_packet() > > adds OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4 [2] > > > > This issue is not seen in master because of this commit - "f6fabcc624 > > ofproto-dpif: Mark packets as "untracked" after call to ct()" [3] > > > > This patch clears the conn track variables after the ct() action. > > > > I suppose we cannot apply this patch to OVS 2.8 branch because it was > > reverted [4] due to some issues. > > > > I think we can solve this problem either with the below fixe or by > setting > > dl_type to proper value when the packet is sent to controller. > > > > *********************************** > > diff --git a/lib/flow.h b/lib/flow.h > > index 6ae5a674d..076ce36f1 100644 > > --- a/lib/flow.h > > +++ b/lib/flow.h > > @@ -947,6 +947,8 @@ pkt_metadata_from_flow(struct pkt_metadata *md, const > > struct flow *flow) > > flow->ct_tp_dst, > > flow->ct_nw_proto, > > }; > > + } else { > > + memset(&md->ct_orig_tuple, 0, sizeof md->ct_orig_tuple); > > } > > } else { > > memset(&md->ct_orig_tuple, 0, sizeof md->ct_orig_tuple); > > ********************************** > > > > Please let me know if this fix makes sense ? Or if there is a better way > to > > solve it ? > > I think that is a reasonable patch. Will you please propose it as a > formal patch? Please include a thorough commit message. >
_______________________________________________ discuss mailing list disc...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-discuss