| Message ID | 1492215949-75081-2-git-send-email-jarno@ovn.org |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Apr 14, 2017 at 05:25:48PM -0700, Jarno Rajahalme wrote: > Use the existing probed support flag for the original direction tuple > to determine if matches on the original direction tuple can be supported. > > Signed-off-by: Jarno Rajahalme <jarno@ovn.org> > --- > ofproto/ofproto-dpif.c | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index c0212f2..25f8adf 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -4122,7 +4122,8 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) > support = &ofproto->backer->support.odp; > ct_state = MINIFLOW_GET_U8(flow, ct_state); > if (support->ct_state && support->ct_zone && support->ct_mark > - && support->ct_label && support->ct_state_nat) { > + && support->ct_label && support->ct_state_nat > + && support->ct_orig_tuple) { > return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0; > } I don't understand the above logic (before or after). Can you explain it? Maybe there needs to be a comment. Thanks, Ben.
> On Apr 17, 2017, at 4:47 PM, Ben Pfaff <blp@ovn.org> wrote: > > On Fri, Apr 14, 2017 at 05:25:48PM -0700, Jarno Rajahalme wrote: >> Use the existing probed support flag for the original direction tuple >> to determine if matches on the original direction tuple can be supported. >> >> Signed-off-by: Jarno Rajahalme <jarno@ovn.org> >> --- >> ofproto/ofproto-dpif.c | 14 +++++++++++++- >> 1 file changed, 13 insertions(+), 1 deletion(-) >> >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c >> index c0212f2..25f8adf 100644 >> --- a/ofproto/ofproto-dpif.c >> +++ b/ofproto/ofproto-dpif.c >> @@ -4122,7 +4122,8 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) >> support = &ofproto->backer->support.odp; >> ct_state = MINIFLOW_GET_U8(flow, ct_state); >> if (support->ct_state && support->ct_zone && support->ct_mark >> - && support->ct_label && support->ct_state_nat) { >> + && support->ct_label && support->ct_state_nat >> + && support->ct_orig_tuple) { >> return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0; >> } > > I don't understand the above logic (before or after). Can you explain > it? Maybe there needs to be a comment. > I guess a comment is needed, how about: /* Do not bother dissecting the flow if the datapath supports all the features we know of. */ Jarno > Thanks, > > Ben.
On Mon, Apr 17, 2017 at 05:10:53PM -0700, Jarno Rajahalme wrote: > > > On Apr 17, 2017, at 4:47 PM, Ben Pfaff <blp@ovn.org> wrote: > > > > On Fri, Apr 14, 2017 at 05:25:48PM -0700, Jarno Rajahalme wrote: > >> Use the existing probed support flag for the original direction tuple > >> to determine if matches on the original direction tuple can be supported. > >> > >> Signed-off-by: Jarno Rajahalme <jarno@ovn.org> > >> --- > >> ofproto/ofproto-dpif.c | 14 +++++++++++++- > >> 1 file changed, 13 insertions(+), 1 deletion(-) > >> > >> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > >> index c0212f2..25f8adf 100644 > >> --- a/ofproto/ofproto-dpif.c > >> +++ b/ofproto/ofproto-dpif.c > >> @@ -4122,7 +4122,8 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) > >> support = &ofproto->backer->support.odp; > >> ct_state = MINIFLOW_GET_U8(flow, ct_state); > >> if (support->ct_state && support->ct_zone && support->ct_mark > >> - && support->ct_label && support->ct_state_nat) { > >> + && support->ct_label && support->ct_state_nat > >> + && support->ct_orig_tuple) { > >> return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0; > >> } > > > > I don't understand the above logic (before or after). Can you explain > > it? Maybe there needs to be a comment. > > > > I guess a comment is needed, how about: > > /* Do not bother dissecting the flow if the datapath supports all the features we know of. */ OK, I understand now. (I don't know whether this is a valuable optimization.) Acked-by: Ben Pfaff <blp@ovn.org>
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index c0212f2..25f8adf 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -4122,7 +4122,8 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) support = &ofproto->backer->support.odp; ct_state = MINIFLOW_GET_U8(flow, ct_state); if (support->ct_state && support->ct_zone && support->ct_mark - && support->ct_label && support->ct_state_nat) { + && support->ct_label && support->ct_state_nat + && support->ct_orig_tuple) { return ct_state & CS_UNSUPPORTED_MASK ? OFPERR_OFPBMC_BAD_MASK : 0; } @@ -4139,6 +4140,17 @@ check_mask(struct ofproto_dpif *ofproto, const struct miniflow *flow) return OFPERR_OFPBMC_BAD_MASK; } + if (!support->ct_orig_tuple && + (MINIFLOW_GET_U8(flow, ct_nw_proto) || + MINIFLOW_GET_U16(flow, ct_tp_src) || + MINIFLOW_GET_U16(flow, ct_tp_dst) || + MINIFLOW_GET_U32(flow, ct_nw_src) || + MINIFLOW_GET_U32(flow, ct_nw_dst) || + !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_src)) || + !ovs_u128_is_zero(MINIFLOW_GET_U128(flow, ct_ipv6_dst)))) { + return OFPERR_OFPBMC_BAD_MASK; + } + return 0; }
Use the existing probed support flag for the original direction tuple to determine if matches on the original direction tuple can be supported. Signed-off-by: Jarno Rajahalme <jarno@ovn.org> --- ofproto/ofproto-dpif.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)