On Thu, Apr 8, 2021 at 4:59 AM Jamal Hadi Salim <j...@mojatatu.com> wrote: > > On 2021-04-07 7:50 p.m., Cong Wang wrote: > > On Wed, Apr 7, 2021 at 8:36 AM Vlad Buslov <vla...@nvidia.com> wrote: > >> > >> Action init code increments reference counter when it changes an action. > >> This is the desired behavior for cls API which needs to obtain action > >> reference for every classifier that points to action. However, act API just > >> needs to change the action and releases the reference before returning. > >> This sequence breaks when the requested action doesn't exist, which causes > >> act API init code to create new action with specified index, but action is > >> still released before returning and is deleted (unless it was referenced > >> concurrently by cls API). > >> > >> Reproduction: > >> > >> $ sudo tc actions ls action gact > >> $ sudo tc actions change action gact drop index 1 > >> $ sudo tc actions ls action gact > >> > > > > I didn't know 'change' could actually create an action when > > it does not exist. So it sets NLM_F_REPLACE, how could it > > replace a non-existing one? Is this the right behavior or is it too > > late to change even if it is not? > > Thats expected behavior for "change" essentially mapping > to classical "SET" i.e. > "create if it doesnt exist, replace if it exists" > i.e NLM_F_CREATE | NLM_F_REPLACE > > In retrospect, "replace" should probably have been just NLM_F_REPLACE > "replace if it exists, error otherwise". > Currently there is no distinction between the two.
This is how I interpret "replace" too, but again it is probably too late to change. > > "Add" is classical "CREATE" i.e "create if doesnt exist, otherwise > error" > > It may be feasible to fix "replace" but not sure how many scripts over > the years are now dependent on that behavior. Right, we probably have to live with it forever. Thanks.
