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  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, 17 Jul 2016 21:19:35 -0700
From:	Alexei Starovoitov <alexei.starovoitov@...il.com>
To:	Jamal Hadi Salim <jhs@...atatu.com>
Cc:	davem@...emloft.net, netdev@...r.kernel.org, daniel@...earbox.net,
	xiyou.wangcong@...il.com, nikolay@...ulusnetworks.com
Subject: Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

On Sun, Jul 17, 2016 at 04:41:24AM -0400, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim <jhs@...atatu.com>
> 
> This action is intended to be an upgrade from a usability perspective
> from pedit (as well as operational debugability).
> Compare this:
> 
> sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
> u32 match ip protocol 1 0xff flowid 1:2 \
> action pedit munge offset -14 u8 set 0x02 \
>     munge offset -13 u8 set 0x15 \
>     munge offset -12 u8 set 0x15 \
>     munge offset -11 u8 set 0x15 \
>     munge offset -10 u16 set 0x1515 \
>     pipe
> 
> to:
> 
> sudo tc filter add dev $ETH parent 1: protocol ip prio 10 \
> u32 match ip protocol 1 0xff flowid 1:2 \
> action skbmod dmac 02:15:15:15:15:15
> 
> Or worse, try to debug a policy with destination mac, source mac and
> etherype. Then make that a hundred rules and you'll get my point.
> 
> In the future common use cases on pedit can be migrated to this action
> (as an example different fields in ip v4/6, transports like tcp/udp/sctp
> etc). For this first cut, this allows modifying basic ethernet header.
> 
> Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
> ---
>  include/net/tc_act/tc_skbmod.h        |  35 +++++
>  include/uapi/linux/tc_act/tc_skbmod.h |  47 +++++++
>  net/sched/Kconfig                     |  11 ++
>  net/sched/Makefile                    |   1 +
>  net/sched/act_skbmod.c                | 245 ++++++++++++++++++++++++++++++++++
>  5 files changed, 339 insertions(+)
>  create mode 100644 include/net/tc_act/tc_skbmod.h
>  create mode 100644 include/uapi/linux/tc_act/tc_skbmod.h
>  create mode 100644 net/sched/act_skbmod.c

I agree with Cong's point that this new action adds 339 lines of kernel
code without adding any new functionality.
It is suppose to solve debugging issues, but it's hard to understand
from commit log.
I can imagine new tc command 'action skbmod dmac 02:15:15:15:15:15' that
uses kernel pedit action undercover, so from user space point of view
the same effect can be achieved by extending iproute2.
What is the reason to add this action to the kernel?
By debug you mean to dump kernel action list and multiple pedits
are hard to decipher? Anything else I'm missing?

Powered by blists - more mailing lists