On 7/19/17, 8:01 AM, "Justin Pettit" <jpet...@ovn.org> wrote:
> 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. I see, so you noticed it affected debug output. I see the hardcoded values passed in 3 functions. Was there any other impact you can foresee ? Any additional test coverage you can recommend ? > 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. If there is a pressing reason to make a temporary fix, then that is fine. Thanks, --Justin _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev