netdev - Re: [PATCH net] net/sched: act_mirred: fix fragment the packet after defrag in act

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 5 Jul 2020 22:31:33 +0800
From:   wenxu <wenxu@...oud.cn>
To:     Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
Cc:     Cong Wang <xiyou.wangcong@...il.com>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>
Subject: Re: [PATCH net] net/sched: act_mirred: fix fragment the packet after
 defrag in act_ct


在 2020/7/4 1:50, Marcelo Ricardo Leitner 写道:
> On Fri, Jul 03, 2020 at 06:19:51PM +0800, wenxu wrote:
>> On 7/3/2020 8:47 AM, Marcelo Ricardo Leitner wrote:
>>> On Thu, Jul 02, 2020 at 02:39:07PM -0700, Cong Wang wrote:
>>>> On Thu, Jul 2, 2020 at 10:32 AM Marcelo Ricardo Leitner
>>>> <marcelo.leitner@...il.com> wrote:
>>>>> On Thu, Jul 02, 2020 at 05:36:38PM +0800, wenxu wrote:
>>>>>> On 7/2/2020 1:33 AM, Cong Wang wrote:
>>>>>>> On Wed, Jul 1, 2020 at 1:21 AM wenxu <wenxu@...oud.cn> wrote:
>>>>>>>> On 7/1/2020 2:21 PM, wenxu wrote:
>>>>>>>>> On 7/1/2020 2:12 PM, Cong Wang wrote:
>>>>>>>>>> On Tue, Jun 30, 2020 at 11:03 PM wenxu <wenxu@...oud.cn> wrote:
>>>>>>>>>>> Only forward packet case need do fragment again and there is no need do defrag explicit.
>>>>>>>>>> Same question: why act_mirred? You have to explain why act_mirred
>>>>>>>>>> has the responsibility to do this job.
>>>>>>>>> The fragment behavior only depends on the mtu of the device sent in act_mirred. Only in
>>>>>>>>>
>>>>>>>>> the act_mirred can decides whether do the fragment or not.
>>>>>>>> Hi cong,
>>>>>>>>
>>>>>>>>
>>>>>>>> I still think this should be resolved in the act_mirred.  Maybe it is not matter with a "responsibility"
>>>>>>>>
>>>>>>>> Did you have some other suggestion to solve this problem?
>>>>>>> Like I said, why not introduce a new action to handle fragment/defragment?
>>>>>>>
>>>>>>> With that, you can still pipe it to act_ct and act_mirred to achieve
>>>>>>> the same goal.
>>>>>> Thanks.  Consider about the act_fagment, There are two problem for this.
>>>>>>
>>>>>>
>>>>>> The frag action will put the ressemble skb to more than one packets. How these packets
>>>>>>
>>>>>> go through the following tc filter or chain?
>>>>> One idea is to listificate it, but I don't see how it can work. For
>>>>> example, it can be quite an issue when jumping chains, as the match
>>>>> would have to work on the list as well.
>>>> Why is this an issue? We already use goto action for use cases like
>>>> vlan pop/push. The header can be changed all the time, reclassification
>>>> is necessary.
>>> Hmm I'm not sure you got what I meant. That's operating on the very
>>> same skb... I meant that the pipe would handle a list of skbs (like in
>>> netif_receive_skb_list). So when we have a goto action with such skb,
>>> it would have to either break this list and reclassify each skb
>>> individually, or the classification would have to do it. Either way,
>>> it adds more complexity not just to the code but to the user as well
>>> and ends up doing more processing (in case of fragments or not) than
>>> if it knew how to output such packets properly. Or am I just
>>> over-complicating it?
>>>
>>> Or, instead of the explicit "frag" action, make act_ct re-fragment it.
>>> It would need to, somehow, push multiple skbs down the remaining
>>> action pipe. It boils down to the above as well.
>>>
>>>>>> When should use the act_fragament the action,  always before the act_mirred?
>>>>> Which can be messy if you consider chains like: "mirred, push vlan,
>>>>> mirred" or so. "frag, mirred, defrag, push vlan, frag, mirred".
>>>> So you mean we should have a giant act_do_everything?
>>> Not at all, but
>>>
>>>> "Do one thing do it well" is exactly the philosophy of designing tc
>>>> actions, if you are against this, you are too late in the game.
>>> in this case a act_output_it_well could do it. ;-)
>> agree, Maybe a act_output_ct action is better?
> Ahm, sorry, that wasn't really my intention here. I meant that I don't
> see an issue with mirred learning how to fragment it and, with that,
> do its action "well" (as subjective as the term can be).
Thanks， I also think It is ok do fragment in the mirred output.
>
>>> Thanks,
>>>   Marcelo
>>>