[net-next,v3,0/7] Allow offloading of UDP NEW connections via act_ct

Message

Vlad Buslov Jan. 19, 2023, 7:50 p.m. UTC

Currently only bidirectional established connections can be offloaded
via act_ct. Such approach allows to hardcode a lot of assumptions into
act_ct, flow_table and flow_offload intermediate layer codes. In order
to enabled offloading of unidirectional UDP NEW connections start with
incrementally changing the following assumptions:

- Drivers assume that only established connections are offloaded and
  don't support updating existing connections. Extract ctinfo from meta
  action cookie and refuse offloading of new connections in the drivers.

- Fix flow_table offload fixup algorithm to calculate flow timeout
  according to current connection state instead of hardcoded
  "established" value.

- Add new flow_table flow flag that designates bidirectional connections
  instead of assuming it and hardcoding hardware offload of every flow
  in both directions.

- Add new flow_table flow flag that marks the flow for asynchronous
  update. Hardware offload state of such flows is updated by gc task by
  leveraging existing flow 'refresh' code.

With all the necessary infrastructure in place modify act_ct to offload
UDP NEW as unidirectional connection. Pass reply direction traffic to CT
and promote connection to bidirectional when UDP connection state
changes to "assured". Rely on refresh mechanism to propagate connection
state change to supporting drivers.

Note that early drop algorithm that is designed to free up some space in
connection tracking table when it becomes full (by randomly deleting up
to 5% of non-established connections) currently ignores connections
marked as "offloaded". Now, with UDP NEW connections becoming
"offloaded" it could allow malicious user to perform DoS attack by
filling the table with non-droppable UDP NEW connections by sending just
one packet in single direction. To prevent such scenario change early
drop algorithm to also consider "offloaded" connections for deletion.

Vlad Buslov (7):
  net: flow_offload: provision conntrack info in ct_metadata
  netfilter: flowtable: fixup UDP timeout depending on ct state
  netfilter: flowtable: allow unidirectional rules
  netfilter: flowtable: allow updating offloaded rules asynchronously
  net/sched: act_ct: set ctinfo in meta action depending on ct state
  net/sched: act_ct: offload UDP NEW connections
  netfilter: nf_conntrack: allow early drop of offloaded UDP conns

 .../ethernet/mellanox/mlx5/core/en/tc_ct.c    |  4 +-
 .../ethernet/netronome/nfp/flower/conntrack.c | 24 ++++++++
 include/net/netfilter/nf_flow_table.h         |  4 +-
 net/netfilter/nf_conntrack_core.c             | 11 ++--
 net/netfilter/nf_flow_table_core.c            | 25 +++++++--
 net/netfilter/nf_flow_table_offload.c         | 17 ++++--
 net/sched/act_ct.c                            | 55 ++++++++++++++-----
 7 files changed, 107 insertions(+), 33 deletions(-)

Comments

Marcelo Ricardo Leitner Jan. 19, 2023, 9:37 p.m. UTC | #1

On Thu, Jan 19, 2023 at 08:50:57PM +0100, Vlad Buslov wrote:
> Currently only bidirectional established connections can be offloaded
> via act_ct. Such approach allows to hardcode a lot of assumptions into
> act_ct, flow_table and flow_offload intermediate layer codes. In order
> to enabled offloading of unidirectional UDP NEW connections start with
> incrementally changing the following assumptions:
> 
> - Drivers assume that only established connections are offloaded and
>   don't support updating existing connections. Extract ctinfo from meta
>   action cookie and refuse offloading of new connections in the drivers.

Hi Vlad,

Regarding ct_seq_show(). When dumping the CT entries today, it will do
things like:

        if (!test_bit(IPS_OFFLOAD_BIT, &ct->status))
                seq_printf(s, "%ld ", nf_ct_expires(ct)  / HZ);

omit the timeout, which is okay with this new patchset, but then:

        if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status))
                seq_puts(s, "[HW_OFFLOAD] ");
        else if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
                seq_puts(s, "[OFFLOAD] ");
        else if (test_bit(IPS_ASSURED_BIT, &ct->status))
                seq_puts(s, "[ASSURED] ");

Previously, in order to be offloaded, it had to be Assured. But not
anymore after this patchset. Thoughts?

  Marcelo

Vlad Buslov Jan. 20, 2023, 6:38 a.m. UTC | #2

On Thu 19 Jan 2023 at 18:37, Marcelo Ricardo Leitner <[e-mail]> wrote:
> On Thu, Jan 19, 2023 at 08:50:57PM +0100, Vlad Buslov wrote:
>> Currently only bidirectional established connections can be offloaded
>> via act_ct. Such approach allows to hardcode a lot of assumptions into
>> act_ct, flow_table and flow_offload intermediate layer codes. In order
>> to enabled offloading of unidirectional UDP NEW connections start with
>> incrementally changing the following assumptions:
>> 
>> - Drivers assume that only established connections are offloaded and
>>   don't support updating existing connections. Extract ctinfo from meta
>>   action cookie and refuse offloading of new connections in the drivers.
>
> Hi Vlad,
>
> Regarding ct_seq_show(). When dumping the CT entries today, it will do
> things like:
>
>         if (!test_bit(IPS_OFFLOAD_BIT, &ct->status))
>                 seq_printf(s, "%ld ", nf_ct_expires(ct)  / HZ);
>
> omit the timeout, which is okay with this new patchset, but then:
>
>         if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status))
>                 seq_puts(s, "[HW_OFFLOAD] ");
>         else if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
>                 seq_puts(s, "[OFFLOAD] ");
>         else if (test_bit(IPS_ASSURED_BIT, &ct->status))
>                 seq_puts(s, "[ASSURED] ");
>
> Previously, in order to be offloaded, it had to be Assured. But not
> anymore after this patchset. Thoughts?

Hi Marcelo,

I know that for some reason offloaded entries no longer display
'assured' flag in the dump. This could be changed, but I don't have a
preference either way and this patch set doesn't modify the behavior.
Up to you and maintainers I guess.

Vlad Buslov Jan. 20, 2023, 6:57 a.m. UTC | #3

On Fri 20 Jan 2023 at 08:38, Vlad Buslov <[e-mail]> wrote:
> On Thu 19 Jan 2023 at 18:37, Marcelo Ricardo Leitner <[e-mail]> wrote:
>> On Thu, Jan 19, 2023 at 08:50:57PM +0100, Vlad Buslov wrote:
>>> Currently only bidirectional established connections can be offloaded
>>> via act_ct. Such approach allows to hardcode a lot of assumptions into
>>> act_ct, flow_table and flow_offload intermediate layer codes. In order
>>> to enabled offloading of unidirectional UDP NEW connections start with
>>> incrementally changing the following assumptions:
>>> 
>>> - Drivers assume that only established connections are offloaded and
>>>   don't support updating existing connections. Extract ctinfo from meta
>>>   action cookie and refuse offloading of new connections in the drivers.
>>
>> Hi Vlad,
>>
>> Regarding ct_seq_show(). When dumping the CT entries today, it will do
>> things like:
>>
>>         if (!test_bit(IPS_OFFLOAD_BIT, &ct->status))
>>                 seq_printf(s, "%ld ", nf_ct_expires(ct)  / HZ);
>>
>> omit the timeout, which is okay with this new patchset, but then:
>>
>>         if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status))
>>                 seq_puts(s, "[HW_OFFLOAD] ");
>>         else if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
>>                 seq_puts(s, "[OFFLOAD] ");
>>         else if (test_bit(IPS_ASSURED_BIT, &ct->status))
>>                 seq_puts(s, "[ASSURED] ");
>>
>> Previously, in order to be offloaded, it had to be Assured. But not
>> anymore after this patchset. Thoughts?
>
> Hi Marcelo,
>
> I know that for some reason offloaded entries no longer display
> 'assured' flag in the dump. This could be changed, but I don't have a
> preference either way and this patch set doesn't modify the behavior.
> Up to you and maintainers I guess.

BTW after checking the log I don't think the assumption that all
offloaded connections are always assured is true. As far as I understand
act_ct originally offloaded established connections and change to
offload assured was made relatively recently in 43332cf97425
("net/sched: act_ct: Offload only ASSURED connections") without
modifying the prints you mentioned.

Marcelo Ricardo Leitner Jan. 20, 2023, 11:30 a.m. UTC | #4

On Fri, Jan 20, 2023 at 08:57:16AM +0200, Vlad Buslov wrote:
> On Fri 20 Jan 2023 at 08:38, Vlad Buslov <[e-mail]> wrote:
> > On Thu 19 Jan 2023 at 18:37, Marcelo Ricardo Leitner <[e-mail]> wrote:
> >> On Thu, Jan 19, 2023 at 08:50:57PM +0100, Vlad Buslov wrote:
> >>> Currently only bidirectional established connections can be offloaded
> >>> via act_ct. Such approach allows to hardcode a lot of assumptions into
> >>> act_ct, flow_table and flow_offload intermediate layer codes. In order
> >>> to enabled offloading of unidirectional UDP NEW connections start with
> >>> incrementally changing the following assumptions:
> >>> 
> >>> - Drivers assume that only established connections are offloaded and
> >>>   don't support updating existing connections. Extract ctinfo from meta
> >>>   action cookie and refuse offloading of new connections in the drivers.
> >>
> >> Hi Vlad,
> >>
> >> Regarding ct_seq_show(). When dumping the CT entries today, it will do
> >> things like:
> >>
> >>         if (!test_bit(IPS_OFFLOAD_BIT, &ct->status))
> >>                 seq_printf(s, "%ld ", nf_ct_expires(ct)  / HZ);
> >>
> >> omit the timeout, which is okay with this new patchset, but then:
> >>
> >>         if (test_bit(IPS_HW_OFFLOAD_BIT, &ct->status))
> >>                 seq_puts(s, "[HW_OFFLOAD] ");
> >>         else if (test_bit(IPS_OFFLOAD_BIT, &ct->status))
> >>                 seq_puts(s, "[OFFLOAD] ");
> >>         else if (test_bit(IPS_ASSURED_BIT, &ct->status))
> >>                 seq_puts(s, "[ASSURED] ");
> >>
> >> Previously, in order to be offloaded, it had to be Assured. But not
> >> anymore after this patchset. Thoughts?
> >
> > Hi Marcelo,
> >
> > I know that for some reason offloaded entries no longer display
> > 'assured' flag in the dump. This could be changed, but I don't have a
> > preference either way and this patch set doesn't modify the behavior.
> > Up to you and maintainers I guess.
> 
> BTW after checking the log I don't think the assumption that all
> offloaded connections are always assured is true. As far as I understand
> act_ct originally offloaded established connections and change to
> offload assured was made relatively recently in 43332cf97425
> ("net/sched: act_ct: Offload only ASSURED connections") without
> modifying the prints you mentioned.

Oh. Somehow this behavior glued to my mind as it was always there. Not
sure which glue was used, please don't ask :D
Thanks!

Reviewed-by: Marcelo Ricardo Leitner <[e-mail]>