Re: WAS ( Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-21 Thread Daniel Borkmann
On 07/21/2016 09:27 AM, Jamal Hadi Salim wrote: [...] Forgot about csum which would work with pedit - didnt quiet parse what you are saying above though: does changing MAC address require changing to CHECKSUM_NONE? If yes, then seems like i need to send a patch for act_ife as well to make in the

WAS ( Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-21 Thread Jamal Hadi Salim
On 16-07-19 08:23 PM, Daniel Borkmann wrote: On 07/19/2016 08:04 PM, Cong Wang wrote: On Tue, Jul 19, 2016 at 8:03 AM, Daniel Borkmann wrote: On 07/19/2016 03:56 PM, Jamal Hadi Salim wrote: [...] [..] I don't think so. 1) checksum is supposed to be done by csum action

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Daniel Borkmann
On 07/19/2016 08:04 PM, Cong Wang wrote: On Tue, Jul 19, 2016 at 8:03 AM, Daniel Borkmann wrote: On 07/19/2016 03:56 PM, Jamal Hadi Salim wrote: [...] But apart from this, neither pedit nor tcf_skbmod_run() here handle checksum complete, so you'll potentially get false

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Cong Wang
On Tue, Jul 19, 2016 at 8:03 AM, Daniel Borkmann wrote: > On 07/19/2016 03:56 PM, Jamal Hadi Salim wrote: > [...] >>> >>> But apart from this, >>> neither pedit nor tcf_skbmod_run() here handle checksum complete, so >>> you'll >>> potentially get false positives wrt csum

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Daniel Borkmann
On 07/19/2016 03:56 PM, Jamal Hadi Salim wrote: [...] But apart from this, neither pedit nor tcf_skbmod_run() here handle checksum complete, so you'll potentially get false positives wrt csum corruption and drops as a result when using either of the two. pedit maybe tricky. Any suggestions? On

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Jamal Hadi Salim
On 16-07-19 09:21 AM, Daniel Borkmann wrote: True, the 32 bit chunks are more generic and as such you need to put more effort in user space to handle them, but at the same time gain more flexibility w/o having to have a module for each and every proto. I dont see anything wrong with using

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Daniel Borkmann
On 07/18/2016 12:08 PM, Jamal Hadi Salim wrote: On 16-07-18 05:44 AM, Daniel Borkmann wrote: On 07/18/2016 08:51 AM, Jamal Hadi Salim wrote: On 16-07-18 12:19 AM, Alexei Starovoitov wrote: Looking at that just out of curiosity on how complex it could look for src/dst mac, is it actually

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-19 Thread Jamal Hadi Salim
On 16-07-18 01:38 PM, Cong Wang wrote: On Mon, Jul 18, 2016 at 3:26 AM, Jamal Hadi Salim wrote: On 16-07-18 06:07 AM, Thomas Graf wrote: Right. I was at the same point as Jamal and it is nasty to try and reverse engineer the dumps without any further hints. I assume

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Cong Wang
On Mon, Jul 18, 2016 at 3:26 AM, Jamal Hadi Salim wrote: > On 16-07-18 06:07 AM, Thomas Graf wrote: > >> >> Right. I was at the same point as Jamal and it is nasty to try and >> reverse engineer the dumps without any further hints. I assume that's >> what he is referring to

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Jamal Hadi Salim
On 16-07-18 06:07 AM, Thomas Graf wrote: Right. I was at the same point as Jamal and it is nasty to try and reverse engineer the dumps without any further hints. I assume that's what he is referring to with difficulties. That +: if you get me a field which says "dstmac" i dont have to go

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Jamal Hadi Salim
On 16-07-18 05:44 AM, Daniel Borkmann wrote: On 07/18/2016 08:51 AM, Jamal Hadi Salim wrote: On 16-07-18 12:19 AM, Alexei Starovoitov wrote: Looking at that just out of curiosity on how complex it could look for src/dst mac, is it actually functional in iproute2 upstream tree? No it is a

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Thomas Graf
On 07/18/16 at 11:44am, Daniel Borkmann wrote: > Same for tcp, icmp, ipv6 bits code ... :/ Is it still planned to eventually > complete these? I agree that from a usability PoV, it might be nice to > have some kind of 'pretty printer' for it besides the existing config > parser there (e.g. when we

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Daniel Borkmann
On 07/18/2016 08:51 AM, Jamal Hadi Salim wrote: On 16-07-18 12:19 AM, Alexei Starovoitov wrote: On Sun, Jul 17, 2016 at 04:41:24AM -0400, Jamal Hadi Salim wrote: From: Jamal Hadi Salim [...] I can imagine new tc command 'action skbmod dmac 02:15:15:15:15:15' that uses

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-18 Thread Jamal Hadi Salim
On 16-07-18 12:19 AM, Alexei Starovoitov wrote: On Sun, Jul 17, 2016 at 04:41:24AM -0400, Jamal Hadi Salim wrote: From: Jamal Hadi Salim This action is intended to be an upgrade from a usability perspective from pedit (as well as operational debugability). Compare this:

Re: [PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-17 Thread Alexei Starovoitov
On Sun, Jul 17, 2016 at 04:41:24AM -0400, Jamal Hadi Salim wrote: > From: Jamal Hadi Salim > > 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:

[PATCH net-next 1/1] net_sched: Introduce skbmod action

2016-07-17 Thread Jamal Hadi Salim
From: Jamal Hadi Salim 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