On Tue, Jun 25, 2019 at 9:30 AM Eyal Birger <eyal.bir...@gmail.com> wrote: > > Hi John, > > On Mon, 24 Jun 2019 23:13:36 +0100 > John Hurley <john.hur...@netronome.com> wrote: > > > TC hooks allow the application of filters and actions to packets at > > both ingress and egress of the network stack. It is possible, with > > poor configuration, that this can produce loops whereby an ingress > > hook calls a mirred egress action that has an egress hook that > > redirects back to the first ingress etc. The TC core classifier > > protects against loops when doing reclassifies but there is no > > protection against a packet looping between multiple hooks and > > recursively calling act_mirred. This can lead to stack overflow > > panics. > > > > Add a per CPU counter to act_mirred that is incremented for each > > recursive call of the action function when processing a packet. If a > > limit is passed then the packet is dropped and CPU counter reset. > > > > Note that this patch does not protect against loops in TC datapaths. > > Its aim is to prevent stack overflow kernel panics that can be a > > consequence of such loops. > > > > Signed-off-by: John Hurley <john.hur...@netronome.com> > > Reviewed-by: Simon Horman <simon.hor...@netronome.com> > > --- > > net/sched/act_mirred.c | 14 ++++++++++++++ > > 1 file changed, 14 insertions(+) > > > > diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c > > index 8c1d736..c3fce36 100644 > > --- a/net/sched/act_mirred.c > > +++ b/net/sched/act_mirred.c > > @@ -27,6 +27,9 @@ > > static LIST_HEAD(mirred_list); > > static DEFINE_SPINLOCK(mirred_list_lock); > > > > +#define MIRRED_RECURSION_LIMIT 4 > > Could you increase the limit to maybe 6 or 8? I am aware of cases where > mirred ingress is used for cascading several layers of logical network > interfaces and 4 seems a little limiting. > > Thanks, > Eyal.
Hi Eyal, The value of 4 is basically a revert to what it was on older kernels when TC had a TTL value in the skb: https://elixir.bootlin.com/linux/v3.19.8/source/include/uapi/linux/pkt_cls.h#L97 I also found with my testing that a value greater than 4 was sailing close to the edge. With a larger value (on my system anyway), I could still trigger a stack overflow here. I'm not sure on the history of why a value of 4 was selected here but it seems to fall into line with my findings. Is there a hard requirement for >4 recursive calls here? John
