netdev - Re: [PATCH 0/4 net-next] flow_offload: update mangle action representation

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]

Message-ID: <20190901134754.1bcd72d4@cakuba.netronome.com>
Date:   Sun, 1 Sep 2019 13:47:54 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Pablo Neira Ayuso <pablo@...filter.org>
Cc:     netfilter-devel@...r.kernel.org, davem@...emloft.net,
        netdev@...r.kernel.org, vishal@...lsio.com, saeedm@...lanox.com,
        jiri@...nulli.us
Subject: Re: [PATCH 0/4 net-next] flow_offload: update mangle action
 representation

On Sat, 31 Aug 2019 16:22:17 +0200, Pablo Neira Ayuso wrote:
> On Fri, Aug 30, 2019 at 03:33:51PM -0700, Jakub Kicinski wrote:
> > On Fri, 30 Aug 2019 11:07:10 +0200, Pablo Neira Ayuso wrote:  
> > > > > * The front-end coalesces consecutive pedit actions into one single
> > > > >   word, so drivers can mangle IPv6 and ethernet address fields in one
> > > > >   single go.    
> > > > 
> > > > You still only coalesce up to 16 bytes, no?    
> > > 
> > > You only have to rise FLOW_ACTION_MANGLE_MAXLEN coming in this patch
> > > if you need more. I don't know of any packet field larger than 16
> > > bytes. If there is a use-case for this, it should be easy to rise that
> > > definition.  
> > 
> > Please see the definitions of:
> > 
> > struct nfp_fl_set_eth
> > struct nfp_fl_set_ip4_addrs
> > struct nfp_fl_set_ip4_ttl_tos
> > struct nfp_fl_set_ipv6_tc_hl_fl
> > struct nfp_fl_set_ipv6_addr
> > struct nfp_fl_set_tport
> > 
> > These are the programming primitives for header rewrites in the NFP.
> > Since each of those contains more than just one field, we'll have to
> > keep all the field coalescing logic in the driver, even if you coalesce
> > while fields (i.e. IPv6 addresses).  
> 
> nfp has been updated in this patch series to deal with the new mangle
> representation.

It has been updated to handle the trivial coalescing.

> > Perhaps it's not a serious blocker for the series, but it'd be nice if
> > rewrite action grouping was handled in the core. Since you're already
> > poking at that code..  
> 
> Rewrite action grouping is already handled from the core front-end in
> this patch series.

If you did what I'm asking the functions nfp_fl_check_mangle_start()
and nfp_fl_check_mangle_end() would no longer exist. They were not
really needed before you "common flow API" changes.

Your reply makes limited amount of sense to me. Pleas read the code and
what I wrote, if you think I'm asking for too much just say that, I'd
accept that.