Hello Jakub,
On Wed, Oct 16, 2019 at 04:36:51PM -0700, Jakub Kicinski wrote:
> Let's see if I can recount the facts:
> (1) this is a "improvement" to simplify driver work but driver
> developers (Ed and I) don't like it;
Ed requested to support for partial mangling of header fields. This
patchset already supports for this, eg. mangle one single byte of a
TCP port.
> (2) it's supposed to simplify things yet it makes the code longer;
The driver codebase is simplified at the cost of adding more frontend
code which is common to everyone.
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c | 162 +++---------
drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h | 40 ---
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 80 ++----
drivers/net/ethernet/netronome/nfp/flower/action.c | 191 ++++++--------
> (3) it causes loss of functionality (looks like a single u32 changing
> both sport and dport is rejected by the IR since it wouldn't
> match fields);
Not correct.
tc filter add dev eth0 protocol ip \
parent ffff: \
pref 11 \
flower ip_proto tcp \
dst_port 80 \
src_ip 1.1.2.3/24 \
action pedit ex munge tcp src 2004 \
action pedit ex munge tcp dst 80
This results in two independent tc pedit actions:
* One tc pedit action with one single key, with value 0xd4070000 /
0x0000ffff.
* Another tc pedit action with one single key, with value 0x00005000
/ 0xffff0000.
This works perfectly with this patchset.
> (4) at v5 it still is buggy (see below).
That, I can fix, thank you for reporting.
> The motivation for this patch remains unclear.
The motivation is to provide a representation for drivers that is
easier to interpret. Have a look at the nfp driver and tell me if it
not easier to follow. This is already saving complexity from the
drivers.
Thank you.
