On Wed, Dec 21, 2016 at 12:02 PM, Daniel Borkmann <dan...@iogearbox.net> wrote: > On 12/21/2016 08:10 PM, Cong Wang wrote: >> >> On Wed, Dec 21, 2016 at 10:51 AM, Cong Wang <xiyou.wangc...@gmail.com> >> wrote: >>> >>> On Wed, Dec 21, 2016 at 9:04 AM, Daniel Borkmann <dan...@iogearbox.net> >>> wrote: >>>> >>>> What happens is that in tc_ctl_tfilter(), thread A allocates a new >>>> tp, initializes it, sets tp_created to 1, and calls into >>>> tp->ops->change() >>>> with it. In that classifier callback we had to unlock/lock the rtnl >>>> mutex and returned with -EAGAIN. One reason why we need to drop there >>>> is, for example, that we need to request an action module to be loaded. >>> >>> >>> Excellent catch! >>> >>> But why do we have to replay the request here? Shouldn't we just return >>> EAGAIN to user-space and let user-space decide to retry or not? >>> Replaying is the root of the evil here. >> >> >> Answer myself: probably due to historical reasons, but still replaying >> inside such a big function is just error-prone, we should promote it >> out: > > > Have no strong opinion, I guess it could be done as a simplification > for net-next, why not, along with moving out the netlink_ns_capable() > check or possibly other things after careful analysis that don't need > to be redone in that circumstance.
It is only slightly bigger than your current one so could fit for -stable too. Also, it could fix all potential problems like this one. Let compiler do the work, not human. ;)
