Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
Thanks Ben for the review. I have fixed the tests (basically, now ip or ipv6 is added to the flows) and submitted a v2 of the patch. Daniel On Wed, Oct 25, 2017 at 7:21 PM, Ben Pfaffwrote: > On Wed, Oct 25, 2017 at 06:50:29PM +0530, Numan Siddique wrote: > > On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchez < > dalva...@redhat.com > > > wrote: > > > > > > > > > > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff wrote: > > > > > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > > >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez > wrote: > > >> > > 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); > > >> > > > >> > This patch seems reasonable too. > > >> > > > >> > I recommend adding a comment above it to explain what's going on, > > >> > because dl_type is not a metadata field and it's confusing to deal > with > > >> > it in a context that's supposed to be all about metadata. I guess > the > > >> > comment would essentially say that dl_type is essential to the > > >> > interpretation of the conntrack metadata. > > >> > > >> Oh, and for this patch too I'd welcome a formal patch proposal. > > >> > > > > > > Done. Thanks a lot Ben. > > > If this get merged, it would be great if we can get it into 2.8 branch > and > > > add a new tag (2.8.2). > > > > > > Thanks!! > > > > > > > Ben, we have submitted both the patches. The patch from Daniel - ( > > https://patchwork.ozlabs.org/patch/830160/) will fix the issue. > > Not sure if this patch https://patchwork.ozlabs.org/patch/830132/ is > > needed any more. > > > > Request to review these patches if possible as RDO is blocked on these > > patches before we can update from OVS 2.7.2 to OVS 2.8(.2) > > I've reviewed both. I wasn't able to immediately apply either one, but > they're obviously moving in the right direction, so I'd appreciate > follow-up from both of you so that we can get them in. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Wed, Oct 25, 2017 at 10:51 PM, Ben Pfaffwrote: > On Wed, Oct 25, 2017 at 06:50:29PM +0530, Numan Siddique wrote: > > On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchez < > dalva...@redhat.com > > > wrote: > > > > > > > > > > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff wrote: > > > > > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > > >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez > wrote: > > >> > > 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); > > >> > > > >> > This patch seems reasonable too. > > >> > > > >> > I recommend adding a comment above it to explain what's going on, > > >> > because dl_type is not a metadata field and it's confusing to deal > with > > >> > it in a context that's supposed to be all about metadata. I guess > the > > >> > comment would essentially say that dl_type is essential to the > > >> > interpretation of the conntrack metadata. > > >> > > >> Oh, and for this patch too I'd welcome a formal patch proposal. > > >> > > > > > > Done. Thanks a lot Ben. > > > If this get merged, it would be great if we can get it into 2.8 branch > and > > > add a new tag (2.8.2). > > > > > > Thanks!! > > > > > > > Ben, we have submitted both the patches. The patch from Daniel - ( > > https://patchwork.ozlabs.org/patch/830160/) will fix the issue. > > Not sure if this patch https://patchwork.ozlabs.org/patch/830132/ is > > needed any more. > > > > Request to review these patches if possible as RDO is blocked on these > > patches before we can update from OVS 2.7.2 to OVS 2.8(.2) > > I've reviewed both. I wasn't able to immediately apply either one, but > they're obviously moving in the right direction, so I'd appreciate > follow-up from both of you so that we can get them in. > Thanks for the review. Numan ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Wed, Oct 25, 2017 at 06:50:29PM +0530, Numan Siddique wrote: > On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchez> wrote: > > > > > > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff wrote: > > > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez wrote: > >> > > 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); > >> > > >> > This patch seems reasonable too. > >> > > >> > I recommend adding a comment above it to explain what's going on, > >> > because dl_type is not a metadata field and it's confusing to deal with > >> > it in a context that's supposed to be all about metadata. I guess the > >> > comment would essentially say that dl_type is essential to the > >> > interpretation of the conntrack metadata. > >> > >> Oh, and for this patch too I'd welcome a formal patch proposal. > >> > > > > Done. Thanks a lot Ben. > > If this get merged, it would be great if we can get it into 2.8 branch and > > add a new tag (2.8.2). > > > > Thanks!! > > > > Ben, we have submitted both the patches. The patch from Daniel - ( > https://patchwork.ozlabs.org/patch/830160/) will fix the issue. > Not sure if this patch https://patchwork.ozlabs.org/patch/830132/ is > needed any more. > > Request to review these patches if possible as RDO is blocked on these > patches before we can update from OVS 2.7.2 to OVS 2.8(.2) I've reviewed both. I wasn't able to immediately apply either one, but they're obviously moving in the right direction, so I'd appreciate follow-up from both of you so that we can get them in. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Wed, Oct 25, 2017 at 3:09 PM, Daniel Alvarez Sanchezwrote: > > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff wrote: > >> On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: >> > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez wrote: >> > > 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); >> > >> > This patch seems reasonable too. >> > >> > I recommend adding a comment above it to explain what's going on, >> > because dl_type is not a metadata field and it's confusing to deal with >> > it in a context that's supposed to be all about metadata. I guess the >> > comment would essentially say that dl_type is essential to the >> > interpretation of the conntrack metadata. >> >> Oh, and for this patch too I'd welcome a formal patch proposal. >> > > Done. Thanks a lot Ben. > If this get merged, it would be great if we can get it into 2.8 branch and > add a new tag (2.8.2). > > Thanks!! > Ben, we have submitted both the patches. The patch from Daniel - ( https://patchwork.ozlabs.org/patch/830160/) will fix the issue. Not sure if this patch https://patchwork.ozlabs.org/patch/830132/ is needed any more. Request to review these patches if possible as RDO is blocked on these patches before we can update from OVS 2.7.2 to OVS 2.8(.2) Thanks Numan Daniel > > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaffwrote: > On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez wrote: > > > 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); > > > > This patch seems reasonable too. > > > > I recommend adding a comment above it to explain what's going on, > > because dl_type is not a metadata field and it's confusing to deal with > > it in a context that's supposed to be all about metadata. I guess the > > comment would essentially say that dl_type is essential to the > > interpretation of the conntrack metadata. > > Oh, and for this patch too I'd welcome a formal patch proposal. > Done. Thanks a lot Ben. If this get merged, it would be great if we can get it into 2.8 branch and add a new tag (2.8.2). Thanks!! Daniel ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Tue, Oct 24, 2017 at 02:27:59PM -0700, Ben Pfaff wrote: > On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez wrote: > > 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); > > This patch seems reasonable too. > > I recommend adding a comment above it to explain what's going on, > because dl_type is not a metadata field and it's confusing to deal with > it in a context that's supposed to be all about metadata. I guess the > comment would essentially say that dl_type is essential to the > interpretation of the conntrack metadata. Oh, and for this patch too I'd welcome a formal patch proposal. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
On Tue, Oct 24, 2017 at 11:07:58PM +0200, Daniel Alvarez Sanchez wrote: > 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); This patch seems reasonable too. I recommend adding a comment above it to explain what's going on, because dl_type is not a metadata field and it's confusing to deal with it in a context that's supposed to be all about metadata. I guess the comment would essentially say that dl_type is essential to the interpretation of the conntrack metadata. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
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 Pfaffwrote: > 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(>ct_orig_tuple, 0, sizeof md->ct_orig_tuple); > > } > > } else { > > memset(>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. > ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack
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(>ct_orig_tuple, 0, sizeof md->ct_orig_tuple); > } > } else { > memset(>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. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev