Description of problem: It currently accepts any value, and when it fails to parse it, it assumes "none" (because NONE is 0). It is probably aggravated by the switching between "-" in "hw-offload" to "_" in "skip_X". There is a warning in there, but it can easily go unnoticed like in https://bugzilla.redhat.com/show_bug.cgi?id=1975085#c22 (This is not the first time I've seen this happening) AFAIK we need ovs running to change that value, so we can't have ovs fail to start. But it would be good if we could improve this somehow. Perhaps block the "set" command to only known values? void tc_set_policy(const char *policy) { if (!policy) { return; } if (!strcmp(policy, "skip_sw")) { tc_policy = TC_POLICY_SKIP_SW; } else if (!strcmp(policy, "skip_hw")) { tc_policy = TC_POLICY_SKIP_HW; } else if (!strcmp(policy, "none")) { tc_policy = TC_POLICY_NONE; } else { VLOG_WARN("tc: Invalid policy '%s'", policy); return; } VLOG_INFO("tc: Using policy '%s'", policy); }
Any news here?
Hi, Yes, I would love to make this a bit more strict with the arguments it takes in. The issue does not lie alone with the tc policy argument. It happens in all other_config configurations A typical ovs set command looks like this ovs-vsctl set Open_vSwitch . other_config:KEY=VALUE The problem is that KEY and VALUE can be any arbitrary string and ovs-vsctl will happily accept it and not throw any warnings. Ideally ovs-vsctl should look up KEY and make sure it is valid and then make sure VALUE is valid for that particular KEY, otherwise throw an error. We know that the "set" command does do some blocking, i.g. if you misspell "other_config" in the above command you will get an error about it being an invalid column. other_config is so well documented at this point with all the key:value pairs that there shouldnt be any reason why we couldnt be any strict about it Let me look how to implement this in the code. We might need to make a lookup table with all the key:value pairs
*** Bug 2208960 has been marked as a duplicate of this bug. ***