> On Jul 19, 2017, at 2:45 AM, Darrell Ball <db...@vmware.com> wrote:
> 
> This change does not seem to be all that useful.
> 
> When rules are constructed, mask and action support do check previously 
> probed support
> which will be ‘TRUE’.
> 
> Another way to see that the below settings are not useful is to set 
> everything to ‘false’ (see below) and run all
> the system tests in userspace, which will all pass.

This change does affect behavior.  For example, upcall debug log messages will 
now show ct_orig_tuple members.  The reason is that dp_netdev_upcall() passes 
those support parameters to odp_flow_key_from_flow(), which thinks that the 
datapath doesn't support those features, so it doesn't print the values.

> By the way, the .max_vlan_headers and .max_mpls_headers fields, which I did 
> not change are pretty big
> numbers and I am fairly sure OVS does not really support that many vlans and 
> labels.
> 
> static struct odp_support dp_netdev_support = {
>    .max_vlan_headers = SIZE_MAX,
>    .max_mpls_depth = SIZE_MAX,
>    .recirc = false,
>    .ct_state = false,
>    .ct_zone = false,
>    .ct_mark = false,
>    .ct_label = false,
>    .ct_state_nat = false,
>    .ct_orig_tuple = false,
>    .ct_orig_tuple6 = false,
> };
> 
> I think it may be better to clean this up. I can do this if you are ok with 
> that; either way is fine with me.

I agree that since the datapath features are probed, we should pass those 
parameters around instead of using these hardcoded values.  However, it was a 
more invasive change than I wanted to make at the time, and I'd want to apply 
this fix regardless.  I was going to add using the probed values to my to-do 
list, but I'm happy if you want to take it.

Thanks,

--Justin


_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to