On 26.09.2016 16:53, Daniel Borkmann wrote: > On 09/26/2016 04:43 PM, Hannes Frederic Sowa wrote: >> On 26.09.2016 03:35, Florian Westphal wrote: >>> Jamal Hadi Salim <j...@mojatatu.com> wrote: >>>> On 16-09-25 02:31 PM, Florian Westphal wrote: >>>>> Shmulik Ladkani <shmulik.ladk...@gmail.com> wrote: >>>>>> We can later address any loop-detection improvements in mirred. >>>>>> WDYT? >>>>> >>>>> You can address this after fixing infamous spinlock recursion hard >>>>> lockup (which has existed forever): >>>>> >>>>> tc qdisc add dev eth0 root handle 1: prio >>>>> tc filter add dev eth0 parent 1: protocol ip u32 match u32 0 0 flowid >>>>> 1:2 action mirred egress redirect dev eth0 >>>>> >>>>> (only do this on toy vm) >>>> >>>> Realize didnt respond to this. Seems very simple to fix: >>>> if skb->dev->ifindex and m->tcfm_dev->ifindex are the >>>> same, then you can drop the packet. >>> >>> Yes, but I think we get same issue when we deal with stacked >>> interfaces, and redirect is to e.g. vlan on top of physical device. >> >> We do have the adjacent upper lists in all netdevices, calculating if a >> mirred actions would insert the skb on a stacked device above us should >> be as easy as querying netdev_has_upper_dev and should be possible to >> check that during config time. > > But that would still not be enough, no? In the sense that with above > scenario, you could redirect to some arbitrary device that redirects > this back to the original device if on purpose configured as such, > thus they don't necessarily need to have a stacked relationship.
Yes, it would only help with the scenario Florian described above. Personally, I would only try to fix and warn against the easy to detect cases. It is easy enough to just create a loop with your local attached L2 which brings your box into a endless loop processing the same packet again and again. Because it is out of control of the kernel you cannot do anything at all. I would just care that we sometimes reschedule and don't do everything in one stack so we don't corrupt the machine and an admin has still a chance to solve the problem. Bye, Hannes
