On 16-09-08 12:11 PM, Eric Dumazet wrote:
On Thu, 2016-09-08 at 09:02 -0700, Eric Dumazet wrote:
On Thu, 2016-09-08 at 06:38 -0400, Jamal Hadi Salim wrote:
On 16-09-06 10:25 AM, Eric Dumazet wrote:
This simply works for me.
But maybe you read the stats at the wrong place ?
Pleas give us mo
On Thu, 2016-09-08 at 09:02 -0700, Eric Dumazet wrote:
> On Thu, 2016-09-08 at 06:38 -0400, Jamal Hadi Salim wrote:
> > On 16-09-06 10:25 AM, Eric Dumazet wrote:
> >
> > >
> > > Just use u16 in the array ?
> > >
> > > u16 tmpaddr[ETH_ALEN / 2]; /* ether_addr_copy() requirement */
> > >
> > > ether
On Thu, 2016-09-08 at 06:38 -0400, Jamal Hadi Salim wrote:
> On 16-09-06 10:25 AM, Eric Dumazet wrote:
>
> >
> > Just use u16 in the array ?
> >
> > u16 tmpaddr[ETH_ALEN / 2]; /* ether_addr_copy() requirement */
> >
> > ether_addr_copy((u8 *)tmpaddr, eth_hdr(skb)->h_dest);
> > ...
> >
>
> Ok - th
On 16-09-06 10:25 AM, Eric Dumazet wrote:
Just use u16 in the array ?
u16 tmpaddr[ETH_ALEN / 2]; /* ether_addr_copy() requirement */
ether_addr_copy((u8 *)tmpaddr, eth_hdr(skb)->h_dest);
...
Ok - thanks Eric. BTW:
Nobody has answered my other question:
I can get stats to increment with:
bs
On Tue, 2016-09-06 at 08:08 -0400, Jamal Hadi Salim wrote:
> ng
> On 16-08-30 08:44 AM, Eric Dumazet wrote:
> > On Tue, 2016-08-30 at 07:57 -0400, Jamal Hadi Salim wrote:
> >> if (flags & SKBMOD_F_SWAPMAC) {
> >> u8 tmpaddr[ETH_ALEN];
> >> /*XXX: I am sure we
ng
On 16-08-30 08:44 AM, Eric Dumazet wrote:
On Tue, 2016-08-30 at 07:57 -0400, Jamal Hadi Salim wrote:
if (flags & SKBMOD_F_SWAPMAC) {
u8 tmpaddr[ETH_ALEN];
/*XXX: I am sure we can come up with something more efficient */
ether_addr_copy(t
On 16-08-30 08:35 AM, Eric Dumazet wrote:
synchronize_rcu() might bee to expensive if you plan to change actions
hundred of times per second.
You could instead add a 'struct rcu_head rcu;' field in struct
tcf_skbmod_params (but make sure this is not exported to user space)
Then :
if
On Tue, 2016-08-30 at 07:57 -0400, Jamal Hadi Salim wrote:
> if (flags & SKBMOD_F_SWAPMAC) {
> u8 tmpaddr[ETH_ALEN];
> /*XXX: I am sure we can come up with something more efficient
> */
> ether_addr_copy(tmpaddr, eth_hdr(skb)->h_dest);
>
On Tue, 2016-08-30 at 07:57 -0400, Jamal Hadi Salim wrote:
> rcu_assign_pointer(d->skbmod_p, p);
> if (ovr) {
> spin_unlock_bh(&d->tcf_lock);
> synchronize_rcu();
> }
>
> kfree(p_old);
Overall patch looks good Jamal, thanks.
synchr
On Tue, 2016-08-30 at 07:12 -0400, Jamal Hadi Salim wrote:
> On 16-08-29 02:20 PM, Eric Dumazet wrote:
>
> >
> > You would need to store everything in an object, managed by rcu.
> >
> > struct my_rcu_safe_struct {
> > u32 flags;
> > u8 eth_dst[ETH_ALEN];
> > u8 eth_src[ETH_ALEN]
On 16-08-30 07:12 AM, Jamal Hadi Salim wrote:
Thanks Eric.
This is the approach i thought Cong was going to take but in a way that
it applies to all actions.
It requires I do this extra allocation per update/create - I am not sure
how much of a big deal that is (we take pride in our update rate
On 16-08-29 02:20 PM, Eric Dumazet wrote:
You would need to store everything in an object, managed by rcu.
struct my_rcu_safe_struct {
u32 flags;
u8 eth_dst[ETH_ALEN];
u8 eth_src[ETH_ALEN];
__be16 eth_type;
};
And then allocate a new one when you need to update the inf
On Mon, 2016-08-29 at 07:35 -0400, Jamal Hadi Salim wrote:
>flags = READ_ONCE(d->flags);
> rcu_read_lock();
> if (flags & SKBMOD_F_DMAC)
> ether_addr_copy(eth_hdr(skb)->h_dest, d->eth_dst);
> if (flags & SKBMOD_F_SMAC)
> ether_addr_cop
On Sun, 2016-08-28 at 22:10 -0700, Cong Wang wrote:
> On Sun, Aug 28, 2016 at 9:07 AM, Eric Dumazet wrote:
> >
> > Adding an action with a spinlock held in fast path in 2016 is
> > a way to tell people : It is a toy, do not use it for real.
> >
> > Sorry guys. Friends do not let friends do that an
On 16-08-29 07:40 AM, Jamal Hadi Salim wrote:
On 16-08-29 07:00 AM, Daniel Borkmann wrote:
Sorry missed that. Let me give it an attempt. I think challenge is
going to be what length to use. For now it is ethernet; but i had
a change which swapped VLANs that i took out.
something like this?
On 16-08-29 06:38 AM, Jamal Hadi Salim wrote:
Let me see if i can
try something. Trickery with RCU is not one of my virtues -
so i may get it wrong but hope you can comment.
Something like attached? Compile tested.
I tried to emulate gact - but greping around I havent
seen sample code which use
On 16-08-29 07:00 AM, Daniel Borkmann wrote:
You could probably just keep these meta data in a separate struct
managed by RCU that tcf_skbmod points to.
Which seem like could be made generic for all actions. Maybe thats
what Cong is thinking.
One more thing I commented on your last patch al
On 08/29/2016 12:38 PM, Jamal Hadi Salim wrote:
On 16-08-28 12:07 PM, Eric Dumazet wrote:
Adding an action with a spinlock held in fast path in 2016 is
a way to tell people : It is a toy, do not use it for real.
Sorry guys. Friends do not let friends do that anymore.
Generally agree.
Cong sa
On 16-08-28 12:07 PM, Eric Dumazet wrote:
Adding an action with a spinlock held in fast path in 2016 is
a way to tell people : It is a toy, do not use it for real.
Sorry guys. Friends do not let friends do that anymore.
Generally agree.
Cong says he is working on generic patches. Let me see i
On 16-08-28 10:48 AM, Alexei Starovoitov wrote:
On Sun, Aug 28, 2016 at 08:19:16AM -0400, Jamal Hadi Salim wrote:
the address is incorrect.
That's why it's recommended to skip this section in all headers.
First paragraph in the above 'This program is ... GPL' would have been enough.
I am su
On Sun, Aug 28, 2016 at 9:07 AM, Eric Dumazet wrote:
>
> Adding an action with a spinlock held in fast path in 2016 is
> a way to tell people : It is a toy, do not use it for real.
>
> Sorry guys. Friends do not let friends do that anymore.
>
Please stop joking, this is not funny at all:
% git g
On Sun, 2016-08-28 at 08:19 -0400, Jamal Hadi Salim wrote:
> From: Jamal Hadi Salim
...
> +static int tcf_skbmod_run(struct sk_buff *skb, const struct tc_action *a,
> + struct tcf_result *res)
> +{
> + struct tcf_skbmod *d = to_skbmod(a);
> +
> + spin_lock(&d->tcf_l
On Sun, Aug 28, 2016 at 08:19:16AM -0400, Jamal Hadi Salim wrote:
> --- /dev/null
> +++ b/include/uapi/linux/tc_act/tc_skbmod.h
> @@ -0,0 +1,49 @@
> +/*
> + * Copyright (c) 2016, Jamal Hadi Salim
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the ter
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 munge offset -14 u8 set
24 matches
Mail list logo
