. Refactor this code to standalone
tcf_block_find function that is used by all three new handlers.
Signed-off-by: Vlad Buslov <vla...@mellanox.com>
FWIW, I like this separation - makes things more maintainable and
readable (we should do it in act_api as well).
cheers,
jamal
dea by Jamal Hadi Salim <j...@mojatatu.com>
Signed-off-by: Qiaobin Fu <qiaob...@bu.edu>
Reviewed-by: Michel Machado <mic...@digirati.com.br>
---
Note that the motivation for this patch is found in the following discussion:
https://www.spinics.net/lists/netdev/msg501061.html
---
diff
t some point when
I get the chance on pedit for implementing the concept of
"copy data to metadata" and "copy metadata to data"
for pedit it made a lot of sense to add the feature there.
In this case skbedit makes more sense.
cheers,
jamal
all action idr spinlock usage with regular calls that do not
disable bh.
Signed-off-by: Vlad Buslov <vla...@mellanox.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
cheers,
jamal
On 21/05/18 10:42 AM, Fu, Qiaobin wrote:
Hi Jamal,
I've tested my patch before publishing it here, and Nishanth is going to test it
further with version 2 of the GKprio. I'm going to push a patch to the repository
iproute2 to add support for "inheritdsfield”.
Thanks. I already
On 17/05/18 03:33 PM, Fu, Qiaobin wrote:
net/sched: add action inheritdsfield to skbmod
The new action inheritdsfield copies the field DS of
IPv4 and IPv6 packets into skb->prioriry. This enables
later classification of packets based on the DS field.
Original idea by Jamal Hadi Salim
cular version of net-next, I will
now rebase my changes on top of it and run them again.
Thank you Vlad!
cheers,
jamal
information (instead of the classical
"We have an error talking to the kernel").
cheers,
jamal
selected to put the packet onto.
The map of this array can be configured from user space. I was saying
earlier that it may be tempting to make a size 64 array to map the
possible dsfields - in practise that has never been pragmatic (so 16 was
sufficient).
cheers,
jamal
if (exists)
+ tcf_idr_release(*a, bind);
return -EPROTONOSUPPORT;
LGTM.
Note: 5026c9b1bafc fixed the bug that existed. It missed a few
spots like the one here. If there was an "Updates" tag, it would
be mor
On 15/05/18 05:21 PM, Vlad Buslov wrote:
On Tue 15 May 2018 at 18:25, Jamal Hadi Salim <j...@mojatatu.com> wrote:
On 14/05/18 04:46 PM, Vlad Buslov wrote:
On Mon 14 May 2018 at 18:03, Jamal Hadi Salim <j...@mojatatu.com> wrote:
On 14/05/18 10:27 AM, Vlad Buslov wrote:
Hello
On 14/05/18 04:46 PM, Vlad Buslov wrote:
On Mon 14 May 2018 at 18:03, Jamal Hadi Salim <j...@mojatatu.com> wrote:
On 14/05/18 10:27 AM, Vlad Buslov wrote:
Hello Jamal,
I'm trying to run tdc, but keep getting following error even on clean
branch without my patches:
Vlad, not sure
the tdc tests with these changes. This area has almost
good test coverage at this point. If you need help just ping me.
cheers,
jamal
Sorry for the latency..
On 09/05/18 01:37 PM, Michel Machado wrote:
On 05/09/2018 10:43 AM, Jamal Hadi Salim wrote:
On 08/05/18 10:27 PM, Cong Wang wrote:
On Tue, May 8, 2018 at 6:29 AM, Jamal Hadi Salim <j...@mojatatu.com>
wrote:
I like the suggestion of extending skbmod to mark s
On 08/05/18 10:27 PM, Cong Wang wrote:
On Tue, May 8, 2018 at 6:29 AM, Jamal Hadi Salim <j...@mojatatu.com> wrote:
Have you considered using skb->prio instead of peeking into the packet
header.
Also have you looked at the dsmark qdisc?
dsmark modifies ds fields, while this one jus
using skb->prio instead of peeking into the packet
header.
Also have you looked at the dsmark qdisc?
cheers,
jamal
s.
cheers,
jamal
From: Jamal Hadi Salim <h...@mojatatu.com>
When a user dumps an existing established tcp socket state
via inet diag, it is possible to retrieve the congestion control
details.
When an the sock is destroyed, the generated event has all the
details available in the dump sans congestion contro
l from outside.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Reviewed-by: Yotam Gigi <yotam...@gmail.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
ortant. Thanks for that!
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
xander Aring <ar...@mojatatu.com>
Reviewed-by: Yotam Gigi <yotam...@gmail.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
Note that the action with index 27 is omitted from the report.
Fixes: 4b3550ef530c ("[NET_SCHED]: Use nla_nest_start/nla_nest_end")"
Signed-off-by: Craig Dillabaugh <cdill...@mojatatu.com>
Good catch.
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 18-03-18 08:06 PM, Davide Caratti wrote:
This series fixes situations where a temporary failure to install a TC
action results in the permanent impossibility to reuse the configured
'index'.
Thanks to Cong Wang for the initial review.
Yikes ;->
These patches look good.
Acked-by: Ja
rg/show_bug.cgi?id=109581
And there is a workaround from Konstantin:
https://patchwork.ozlabs.org/patch/803885/
Unfortunately I don't think that is a real fix, we probably need to
fix HFSC itself rather than just workaround the qlen==0. It is not
trivial since HFSC implementation is not easy to under
/listinfo/people)
If twitter is your thing then follow us: @netdev01
and use hashtag #netdevconf
cheers,
jamal(on behalf of NetDev Society)
of pushing the boundaries saying this targets net tree - there is
no bug you are fixing (as you say the sk is freed).
Maybe it makes the code prettier ...
cheers,
jamal
On 18-02-15 10:54 AM, Alexander Aring wrote:
Hi,
this patch series adds extack support for the TC action subsystem.
As example I for the extack support in a TC action I choosed mirred
action.
For the patch series:
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
support now.
I know there are patches around which makes changes to these files.
I will rebase my stuff on Jiri's patches if they get in before mine.
Given you rebased on top of Jiri's patches already - last two sentences
are unneeded?
cheers,
jamal
One more comment:
Probably try to run a test with a very small delta with
no offload (probably using something like prio as the root qdisc)
and dump the stats.
My gut feeling is your accounting of the backlog in particular is off.
cheers,
jamal
On 18-01-18 08:35 AM, Jamal Hadi Salim wrote
3)It would be helpful for debugging to increment some stats other
than drop counters on enqueu/dequeue obsolete packet drop. Maybe use
overlimits for the dequeu drops (in addition)?
4) I may be misreading things - but did you need to reset the
watchdog on dequeue? It is already being kicked for every incoming packet.
cheers,
jamal
can avoid collission
when we start sending such patches by having the discussion now.
cheers,
jamal
e and it doesnt make mistakes
in this case i.e we will never see this error with tc because a
create will always have those two set correctly; OTOH, a developer
writing some new app is more likely to make this mistake (in which
case this message is very helpful).
cheers,
jamal
On 18-01-16 10:33 AM, Jiri Pirko wrote:
From: Jiri Pirko <j...@mellanox.com>
For patches 1-9:
Reviewed-by: Jamal Hadi Salim <j...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 18-01-16 05:41 PM, Jakub Kicinski wrote:
On Tue, 16 Jan 2018 17:12:57 -0500, Jamal Hadi Salim wrote:
On 18-01-16 04:46 PM, Jakub Kicinski wrote:
On Tue, 16 Jan 2018 12:20:19 -0500, Alexander Aring wrote:
[..]
I would say precedence should be Jiri's patches, Alex's patches
and then yours:
Alex's patches fix the core (cls_api.c) area with proper extack
for the core and then he has one patch to cover a specific
use case of the u32 classifier extack. Yours is only concerned
with one use case - bpf which depend on the core (that is in Alex's
patches)
cheers,
jamal
apping of DSCP to skb prio and by 1:1 PCP values mapping) and up to 8
bands.
Patches 1-2 offload DSCP to priority mapping in the mlxsw_sp driver.
Patch 3 adds offload support for PRIO qdisc.
Patches 4-5 Add PRIO offload support in the mlxsw_sp driver.
Strictly DSCP or 802.1p as well?
cheers,
jamal
...@mojatatu.com wrote:
I only looked at the kernel code. Good you can stop it at tc
but the API does not stop it (unless you expect the rest of the
world to only use tc).
Jamal, apparently, you did not looked at the kernel code either :)
Look at the changes done in net/sched/sch_ingress.c
it (unless you expect the rest of the
world to only use tc).
Jamal, apparently, you did not looked at the kernel code either :)
Look at the changes done in net/sched/sch_ingress.c - there is where the
parsing of block attr takes place.
reason i raised it is from looking at tc_ctl_tfilter
didnt.
Jamal, again, other qdiscs does not support block sharing. This patchset
only adds support for sharing of block for ingress and clsact qdiscs.
Later on, other qdiscs could also support block sharing.
Can you stop a config which says:
tc qdisc add dev ens9 root block 22 handle 1:0 prio ?
qdisc instances to share
filter blocks. The block index is coming from userspace as qdisc option.
Didnt quiet follow why ingress is special and needs attributes to
set the block but other qdiscs didnt.
Jamal, again, other qdiscs does not support block sharing. This patchset
only adds support for s
quiet follow why ingress is special and needs attributes to
set the block but other qdiscs didnt.
Will check again later after some coffee..
cheers,
jamal
On 18-01-09 09:07 AM, Jiri Pirko wrote:
From: Jiri Pirko <j...@mellanox.com>
Add simple block get operation which primary purpose is to check the
block existence by block index.
block_dump missing?
cheers,
jamal
x of 0 instead of TCM_IFINDEX_MAGIC_BLOCK
cheers,
jamal
re about queues (so it may never be "fixed")
cheers,
jamal
On 18-01-07 09:28 AM, Jamal Hadi Salim wrote:
On 18-01-07 08:46 AM, Jiri Pirko wrote:
Sun, Jan 07, 2018 at 02:11:19PM CET, j...@mojatatu.com wrote:
On 18-01-06 03:43 PM, Jiri Pirko wrote:
@@ -886,8 +887,13 @@ static int tcf_fill_node(struct net *net,
struct sk_buff *skb,
tcm
From: Jamal Hadi Salim <j...@mojatatu.com>
Fixes: 249284ff5a44 ("tc: jsonify filter core")
Signed-off-by: Jamal Hadi Salim <j...@mojatatu.com>
---
tc/tc_filter.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/tc_filter.c b/tc/tc_filter.c
index 5
it?
I cant point to any such code, it is just the ifindex is an int.
And the negative space looks like less likely someone would think
of using for signalling (0x as an example).
tcpdump -i any probably assumes soem weird ifindex (havent looked
at the code).
In any case, 0 is fine too.
cheers,
jamal
tive ifindex
(not sure if zero means something speacial to someone).
cheers,
jamal
On 18-01-06 01:02 PM, Jamal Hadi Salim wrote:
On 18-01-06 04:48 AM, Jiri Pirko wrote:
BTW: From your output, DavidA, i noticed something strange:
two flower filters with the same handle id 0x1 (different prios)
At least on the kernel i am using this is the exhibited default
behavior. I can
ssible to bind that block to many parent locations
eg clsact ingress of dev x and clsact egress of dev y.
what am i missing?
cheers,
jamal
f same kind on the same chain
should be distinguished by priority. There are filters like u32
(which hide hash tables under the same priority) which may
allow the same prio for multiple handles - just dont see that
fit with flower, but maybe missing something.
cheers,
jamal
.
clsact egress does not deal with tcf_result - what code do you want
me to check?
cheers,
jamal
,
jamal
f the block.
cheers,
jamal
than the current set you posted Jiri.
A simple solution is to say sharing only works for ingress
(but that sounds very lame).
cheers,
jamal
If this knob was set
then you reject addition of filters via a port/qdisc which is sharing.
I agree with Jiri - if you consciously choose to share there should
be no suprises with what filters get applied.
I think this email is too long and my egress concern will be lost
if i typed it here; so i will
Saw some tiny typos in the commit log but lets please not hold
back the submission so we can have the rest of the
(many) outstanding patches coming in. So for this patch set:
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 17-12-18 05:44 PM, Alexander Aring wrot
On 17-12-06 11:08 AM, Alexander Aring wrote:
This patch adds extack support for graft callback to prepare per-qdisc
specific changes for extack.
Cc: David Ahern<dsah...@gmail.com>
Signed-off-by: Alexander Aring<ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 17-12-06 11:08 AM, Alexander Aring wrote:
This patch adds extack support for block callback to prepare per-qdisc
specific changes for extack.
Cc: David Ahern<dsah...@gmail.com>
Signed-off-by: Alexander Aring<ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 17-12-06 11:08 AM, Alexander Aring wrote:
This patch adds extack support for class change callback api. This prepares
to handle extack support inside each specific class implementation.
Cc: David Ahern<dsah...@gmail.com>
Signed-off-by: Alexander Aring<ar...@mojatatu.com>
Ack
On 17-12-06 11:08 AM, Alexander Aring wrote:
This patch adds extack support for change callback for qdisc ops
structtur to prepare per-qdisc specific changes for extack.
Cc: David Ahern<dsah...@gmail.com>
Signed-off-by: Alexander Aring<ar...@mojatatu.com>
Acked-by: Jamal H
On 17-12-06 11:08 AM, Alexander Aring wrote:
This patch adds extack support for init callback to prepare per-qdisc
specific changes for extack.
Cc: David Ahern<dsah...@gmail.com>
Signed-off-by: Alexander Aring<ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
if init callback is set or not. This patch has the same
behaviour as before, just without assign err variable in if condition.
It also makes the code easier to read.
Reviewed-by: Jamal Hadi Salim <j...@mojatatu.com>
Cc: David Ahern <dsah...@gmail.com>
Signed-off-by: Alexander Aring <ar.
com>
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
On 17-11-23 11:36 AM, Jiri Pirko wrote:
Sat, Apr 22, 2017 at 02:36:23PM CEST, j...@mojatatu.com wrote:
[...]
[..]
Jamal, is there any particular reason that you print cookie only in case
you show stats? What is the relation between cookie and stats?
-s has been thus far used to imply
While we are still gathering slides/papers/videos, folks can take a
look at the reports on the conference on lwn. Netdev 2.2 day 2 and
3 should appear in the next day or so.
https://lwn.net/Articles/738912/
Thanks to lwn for hosting these reports.
Enjoy!
cheers,
jamal
Will leave it up to Dave - but even this + patch 3 should start with
"net_sched actions: act_vlan" as the prefix. Sorry, shouldve said that
in the last comment I sent.
cheers,
jamal
Manish,
A better subject would be:
"[PATCH net-next v8 0/3] net_sched actions: act_vlan use the RCU
I would suggest a v9 with this change.
cheers,
jamal
e container at the end of the
test.
cheers,
jamal
forgot to cc netdev...
cheers,
jamal
Forwarded Message
Subject: bug report: iproute2 policer parsing broken
Date: Sun, 22 Oct 2017 10:59:07 -0400
From: Jamal Hadi Salim <j...@mojatatu.com>
To: Phil Sutter <p...@nwl.cc>, Stephen Hemminger
<step...@networkplu
From: Jamal Hadi Salim <j...@mojatatu.com>
Seems like my old patches didnt make it into the tree - so here goes
Sample use case:
... add ingress qdisc
sudo $TC qdisc add dev $ETH ingress
... if we exceed rate of 1kbps (burst of 90K), do an absolute jump of 2 actions
sudo $TC actio
proach and discussion outcome goes into
net-next?
cheers,
jamal
.
To test if the issue was fixed, apply patches 1 and 2 and
then repeat the tests.
--
Other than that all look good to me and:
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
an _event_
that the vlan has been deleted (when it never existed in the first
place).
This has been the behaviour forever and some script might depend on it.
Also IMO, and as David also mentioned, doing a partial delete is not good.
I think this is a bug (especially the event part).
cheers,
jamal
On 17-10-11 05:16 PM, Alexander Aring wrote:
This patch changes the parameter updating via RCU and not protected by a
spinlock anymore. This reduce the time that the spinlock is being held.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@moj
On 17-10-11 05:16 PM, Alexander Aring wrote:
This patch migrates the current counter handling which is protected by a
spinlock to a per-cpu counter handling. This reduce the time where the
spinlock is being held.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi
option than this.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
classid :2
eth_type ipv4
ip_proto udp
dst_ip 192.168.1.1
dst_port 22
skip_sw
in_hw
Much much better semantic. Thank you.
Have you tested many filter mapping to the same classid?
cheers,
jamal
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
f_idr_create(tn, parm->index, est, a,
-_vlan_ops, bind, false);
+ _vlan_ops, bind, true);
Indentation mismatch here?
Otherwise looks good to me.
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
showing up on the
bridge link even though it is not enslaved there..
cheers,
jamal
Can you please paste a sample default detailed output with your patch
with a few hundred vlans ?
_H_INGRESS(0xFFF1U)
#define TC_H_CLSACT TC_H_INGRESS
#define TC_H_MIN_INGRESS0xFFF2U
#define TC_H_MIN_EGRESS 0xFFF3U
-
You should be able to say add a location which maps to a pre-routing
or post-routing etc; and this would work as well...
cheers,
jamal
f9f, I've tried to
include proper fixes tags in each patch.
I haven't included individual patch acks in the set, I'd appreciate it if
you take another look and resend them.
Hi Nik,
For all patches:
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
Would you please consider adding all the the
>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
Also already acked this before but you left it out in this
version. If you make changes to the patch then you will need
a new ACK.
Dont forget to update selftests please.
cheers,
jamal
a unique action, that is another bottleneck.
Follow-up patch in this patchset addresses that.
Signed-off-by: Chris Mi <chr...@mellanox.com>
Signed-off-by: Jiri Pirko <j...@mellanox.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
BTW: I'd already acked this before b
if (matches(*argv, "dst") == 0) {
NEXT_ARG();
For iproute commands the show output is supposed to match the corresponding set
inputs.
These were being printed at parse time ...
cheers,
jamal
On 17-08-28 03:07 PM, Alexander Aring wrote:
This patch updates the tc-ife man page that the default IFE ethertype
will be used if it's not specified.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
Same comment as previous patch.
cheers,
jamal
On 17-08-28 03:07 PM, Alexander Aring wrote:
This patch will report about if the ethertype for IFE is not specified
that the default IFE type is used.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
---
tc/m_ife.c | 2 ++
1 file chan
Alex,
I think we should get rid of these fprintfs instead of fixing them.
They were originally intended to be debug outputs.
cheers,
jamal
On 17-08-28 03:07 PM, Alexander Aring wrote:
This patch uses the usually IEEE format to display an ethertype which is
4-digits and every digit in upper
On 17-08-28 03:07 PM, Alexander Aring wrote:
This patch allows to set an ethertype for IFE which is zero. There is no
kernel side validation which forbids a type to zero.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
We should add the tests which do filter binding as well
(both auto-binding and late binding)
cheers,
jamal
On 17-08-28 03:03 PM, Alexander Aring wrote:
This patch handles a default IFE type if it's not given by user space
netlink api. The default IFE type will be the registered ethertype by
IEEE for IFE ForCES.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Sa
"ForCES" instead of "FoRCES" which
is a spelling error inside the IEEE ethertype specification.
Signed-off-by: Alexander Aring <ar...@mojatatu.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
cheers,
jamal
a unique action, that is another bottleneck.
Follow-up patch in this patchset addresses that.
Signed-off-by: Chris Mi <chr...@mellanox.com>
Signed-off-by: Jiri Pirko <j...@mellanox.com>
Acked-by: Jamal Hadi Salim <j...@mojatatu.com>
As Cong asked last time - any plans to add to
can just compare the pointers when collision.
And this only affects slow paths, has no impact to fast path,
thanks to the pointer ->tp_c.
Cc: Jamal Hadi Salim <j...@mojatatu.com>
Cc: Jiri Pirko <j...@resnulli.us>
Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
Nice work.
the last refcnt into ->delete(),
right after releasing tree lock. This is fine because the class is
already removed from hash when holding the lock.
For those who also use ->put() as ->unbind(), just rename them to reflect
this change.
Cc: Jamal Hadi Salim <j...@mojatatu.com>
Signe
On 17-08-24 07:51 PM, Cong Wang wrote:
Like for TC actions, ->delete() is a special case,
we have to prepare and fill the notification before delete
otherwise would get use-after-free after we remove the
reference count.
Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com>
Acked-by: J
101 - 200 of 2031 matches
Mail list logo
