On Tue, Oct 24, 2017 at 11:35 PM, Ben Pfaff <b...@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!!
Daniel
_______________________________________________
discuss mailing list
disc...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-discuss
