[resend] Solution for the problem conntrack in tc subsystem

[resend] Solution for the problem conntrack in tc subsystem

[wenxu]

Hi,


Currently kernel tc subsystem can do conntrack things in cat_ct. But there is a problem need

to fix.  For several fragment packets handle in act_ct.  The tcf_ct_handle_fragments will defrag

the packets to a big one. But the after action will redirect mirror to a device which maybe lead

the reassembly big packet over the mtu of target device.


The proposal "net/sched: act_mirred: fix fragment the packet after defrag in act_ct"

http://patchwork.ozlabs.org/project/netdev/patch/1593485646-14989-1-git-send-email-wenxu@ucloud.cn/

is not been accepted.


another proposal "net/sched: add act_ct_output support"

http://patchwork.ozlabs.org/project/netdev/patch/1598335663-26503-1-git-send-email-wenxu@ucloud.cn/

is also not a good solution. There are some duplicate codes for this and act_mirred.


Something other proposal like add the  act_fragment also can't be work.  The fragment will make

The big packet to several ones and can't be handle in the tc pipe. And also this is not friendly for

user to explicitly add action for fragment.

Only do gso for the reassembly big packet is also can't fix all the case such for icmp packet.


So there are some proper solution for this problem. In the Internet we can't avoid the fragment packets.



BR

wenxu

Re: [resend] Solution for the problem conntrack in tc subsystem

[Marcelo Ricardo Leitner]

Cc'ing Cong, Paul, Oz and Davide.

On Thu, Oct 29, 2020 at 10:22:04AM +0800, wenxu wrote:
> Only do gso for the reassembly big packet is also can't fix all the
> case such for icmp packet.

Good point. And as we can't know that a fragment was for an icmp
packet before defraging it, this is quite impactful.

> 
> So there are some proper solution for this problem. In the Internet
> we can't avoid the fragment packets.

I agree. One other idea is to add support for some hook to mirred,
that gets executed before xmiting the packet. Then, when act_ct (or
another specific act module, say act_frag, as act_ct might not be the
only one interested in defragging in the future) gets loaded, it
configs that hook.

So that mirred would something like:
if (xmit_hook)
	xmit_hook(skb, dev_queue_xmit);
else
	dev_queue_xmit(skb);
Even protect it with a static branch key.

This leaves mirred almost untouched, 0 performance penalty for those
that don't use act_ct, can even have a Kconfig knob, is not CT or
ipfrag specific code on mirred so it's reusable later on and solves
our problem here. Thoughts?

  Marcelo

Re: [resend] Solution for the problem conntrack in tc subsystem

[Marcelo Ricardo Leitner]

On Thu, Oct 29, 2020 at 07:59:36PM -0300, Marcelo Ricardo Leitner wrote:
> Cc'ing Cong, Paul, Oz and Davide.
> 
> On Thu, Oct 29, 2020 at 10:22:04AM +0800, wenxu wrote:
> > Only do gso for the reassembly big packet is also can't fix all the
> > case such for icmp packet.
> 
> Good point. And as we can't know that a fragment was for an icmp
> packet before defraging it, this is quite impactful.

Doh, we can, as that info is always present on the iphdr, but still,
quite impactful, as we would have to have special rule sets to cope
with it.

  Marcelo

Re: [resend] Solution for the problem conntrack in tc subsystem

[wenxu]

On 10/30/2020 6:59 AM, Marcelo Ricardo Leitner wrote:
> Cc'ing Cong, Paul, Oz and Davide.
>
> On Thu, Oct 29, 2020 at 10:22:04AM +0800, wenxu wrote:
>> Only do gso for the reassembly big packet is also can't fix all the
>> case such for icmp packet.
> Good point. And as we can't know that a fragment was for an icmp
> packet before defraging it, this is quite impactful.
>
>> So there are some proper solution for this problem. In the Internet
>> we can't avoid the fragment packets.
> I agree. One other idea is to add support for some hook to mirred,
> that gets executed before xmiting the packet. Then, when act_ct (or
> another specific act module, say act_frag, as act_ct might not be the
> only one interested in defragging in the future) gets loaded, it
> configs that hook.
>
> So that mirred would something like:
> if (xmit_hook)
> 	xmit_hook(skb, dev_queue_xmit);
> else
> 	dev_queue_xmit(skb);
> Even protect it with a static branch key.
>
Good idea, The act _mirror almost untouched.

The fragment function can be put in a common place for shared

by other modules.

>
>   Marcelo
>