On 8/25/2020 11:33 PM, Marcelo Ricardo Leitner wrote: > On Tue, Aug 25, 2020 at 02:07:43PM +0800, we...@ucloud.cn wrote: > ... >> +static LIST_HEAD(ct_output_list); >> +static DEFINE_SPINLOCK(ct_output_list_lock); >> + >> +#define CT_OUTPUT_RECURSION_LIMIT 4 >> +static DEFINE_PER_CPU(unsigned int, ct_output_rec_level); > Wenxu, first of all, thanks for doing this. > > Hopefully this helps to show how much duplicated code this means. > Later on, any bug that we find on mirrer, we also need to fix in > act_ct_output, which is not good. > > Currently act_ct is the only one doing defrag and leading to this > need, but that may change in the future. The action here, AFAICT, has > nothing in specific to conntrack. It is "just" re-fragmenting > packets. The only specific reference to nf/ct I could notice is for > the v6ops, to have access to ip6_fragment(), which can also be done > via struct ipv6_stub (once added there). That said, it shouldn't be > named after conntrack, to avoid future confusions.
Yes, there is no direct relation with conntrack. The re-fragmenting based on the qdisc_skb_cb(skb)->mru which only setting in the act_ct. So I named this to ct_xx. > > I still don't understand Cong's argument for not having this on > act_mirred because TC is L2. That's actually not right. TC hooks at L2 > but deals with L3 and L4 (after all, it does static NAT, mungles L4 > headers and classifies based on virtually anything) since beginning, > and this is just another case. > > What I can understand, is that this feature shouldn't be enabled by > default on mirred. So that we are sure that users opting-in know what > they are doing. It can have a "l3" flag, to enable L3 semantics, and > that's it. Code re-used, no performance drawback for pure L2 users (it > can even be protected by a static_key. Once a l3-enabled mirred is > loaded, enable it), user knows what to expect and no confusion on > which action to use. I think it is better put this feature to act_mirred. And it also not enable by default. Only packet with qdisc_skb_cb(skb)->mru which means this packet just do reassemble before conntrack. So only othersolution for solving this problem? BR wenxu > > Marcelo >
