On 17-04-19 11:03 AM, Wolfgang Bumiller wrote:
On April 19, 2017 at 1:32 PM Jamal Hadi Salim <j...@mojatatu.com> wrote:
On 17-04-19 04:09 AM, Wolfgang Bumiller wrote:
This solves one issue, but I am afraid the issue Cong mentioned is a
possibility still.
Lets say user did a replace and tried to also replace the cookie
in that transaction. The init() succeeds but the cookie allocation
fails. To be correct we'll have to undo the replace i.e something
like uninit() which will restore back the old values.
This is very complex and unnecessary.
My suggestion:
If we move the cookie allocation before init we can save it and
only when init succeeds do we attach it to the action, otherwise
we free it on error path.
Shouldn't the old code have freed an old a->act_cookie as well before
replacing it then? nla_memdup_cookie() starts off by assigning a->act_cookie.
(I've been running this loop for a while now:
# while : ; do tc actions change action ok index 500 cookie $i; let i++; done
and memory usage *seems* to be growing faster with the loop running - still
slow, but visible. (I stopped most but not all background processes in this
VM))
I don't see the growth with the change below (replacing both patches).
(although given the freeing of the old act_cookie pointer I wonder if
this needs additional locking?)
I think we are safe. The cookie should not be touched by any datapath
code.
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
