[ovs-dev] [ovs-discuss] [OVN] OVN doesn't work using OVS 2.8.1 on Centos 7.3 using conntrack

Numan Siddique nusiddiq at redhat.com
Wed Oct 25 18:04:47 UTC 2017


On Wed, Oct 25, 2017 at 10:51 PM, Ben Pfaff <blp at ovn.org> wrote:

> 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 <
> dalvarez at redhat.com
> > > wrote:
> >
> > >
> > >
> > > On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff <blp at ovn.org> 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


More information about the dev mailing list