* Re: [PATCH net-next,v2] net: rename ndo_setup_tc to ndo_setup_offload
From: Pablo Neira Ayuso @ 2018-07-19 20:52 UTC (permalink / raw)
To: Alexander Duyck
Cc: Netdev, Jiri Pirko, Tom Lendacky, Florian Fainelli, Ariel Elior,
Michael Chan, Santosh Raspatur, madalin.bucur, yisen.zhuang,
salil.mehta, Jeff Kirsher, Tariq Toukan, Saeed Mahameed,
Jiri Pirko, Ido Schimmel, Ganesh Goudar, Jakub Kicinski,
linux-net-drivers, peppe.cavallaro, alexandre.torgue, joabreu,
grygorii.strashko, Andrew Lunn
In-Reply-To: <CAKgT0UcTihfkgzO=AZ=J_hS-3gg_wjMYr+kJGAWobQ_Wkdfp9g@mail.gmail.com>
On Thu, Jul 19, 2018 at 08:18:20AM -0700, Alexander Duyck wrote:
> On Wed, Jul 18, 2018 at 5:11 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > One of the recurring complaints is that we do not have, as a driver
> > writer, a central location from which we would be fed offloading rules
> > into a NIC. This was brought up again during Netconf'18 in Boston.
> >
> > This patch just renames ndo_setup_tc to ndo_setup_offload as a very
> > early initial work to prepare for follow up patch that discuss unified
> > flow representation for the existing offload programming APIs.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> One request I would have here is to not bother updating the individual
> driver function names. For now I would say we could leave the
> "_setup_tc" in the naming of the driver functions itself and just
> update the name of the net device operation. Renaming the driver
> functions just adds unnecessary overhead and complexity to the patch
> and will make it more difficult to maintain. When we get around to
> adding additional functionality that relates to the rename we could
> address renaming the function on a per driver basis in the future.
Plan was to follow up patch will rename enum tc_setup_type too:
https://marc.info/?l=linux-netdev&m=153193158512556&w=2
that will result in more renames in the driver side.
I would expect this will happen sooner or later, and out of tree
patches will end up needing a rebase sooner or later, if that is the
concern.
^ permalink raw reply
* Re: [PATCH 4/5] net: add support for nvmem to eth_platform_get_mac_address()
From: Bartosz Golaszewski @ 2018-07-19 21:24 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Sekhar Nori, Kevin Hilman, Grygorii Strashko, David S . Miller,
Srinivas Kandagatla, Lukas Wunner, Rob Herring, Florian Fainelli,
Dan Carpenter, Ivan Khoronzhuk, David Lechner, Greg Kroah-Hartman,
Andrew Lunn, Linux ARM, Linux Kernel Mailing List, Linux-OMAP,
netdev, Bartosz Golaszewski
In-Reply-To: <20180719174728.GK6920@n2100.armlinux.org.uk>
2018-07-19 19:47 GMT+02:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> On Wed, Jul 18, 2018 at 06:10:34PM +0200, Bartosz Golaszewski wrote:
>> @@ -544,6 +548,31 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
>> from = "arch callback";
>> }
>>
>> + if (!addr) {
>> + nvmem = nvmem_cell_get(dev, "mac-address");
>> + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
>
> This is way too verbose. To quote Al Viro from earlier today:
>
> <viro> sigh...
> <viro> if (IS_ERR(link) && PTR_ERR(link) == -EEXIST)
> <viro> what the hell is wrong with if (link == ERR_PTR(-EEXIST))?
>
> I wonder why so many people haven't heard of pointer comparison... ;)
>
> IS_ERR(ERR_PTR(-EPROBE_DEFER)) is always true - if it wasn't, then
> we'd be in for problems.
>
> So, if you're asserting that nvmem is ERR_PTR(-EPROBE_DEFER) then
> there's no need to do the IS_ERR(nvmem) must also be true. Hence, a
> simple pointer comparison is sufficient:
>
> if (nvmem == ERR_PTR(-EPROBE_DEFER))
> return -EPROBE_DEFER;
>
Hi Russell,
this issue is gone now in v3 but thanks for the example - I somehow
never thought about it.
Bart
^ permalink raw reply
* [PATCH net] net/ipv6: Fix linklocal to global address with VRF
From: dsahern @ 2018-07-19 19:41 UTC (permalink / raw)
To: netdev; +Cc: davem, David Ahern
From: David Ahern <dsahern@gmail.com>
Example setup:
host: ip -6 addr add dev eth1 2001:db8:104::4
where eth1 is enslaved to a VRF
switch: ip -6 ro add 2001:db8:104::4/128 dev br1
where br1 only has an LLA
ping6 2001:db8:104::4
ssh 2001:db8:104::4
(NOTE: UDP works fine if the PKTINFO has the address set to the global
address and ifindex is set to the index of eth1 with a destination an
LLA).
For ICMP, icmp6_iif needs to be updated to check if skb->dev is an
L3 master. If it is then return the ifindex from rt6i_idev similar
to what is done for loopback.
For TCP, restore the original tcp_v6_iif definition which is needed in
most places and add a new tcp_v6_iif_l3_slave that considers the
l3_slave variability. This latter check is only needed for socket
lookups.
Fixes: 9ff74384600a ("net: vrf: Handle ipv6 multicast and link-local addresses")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
Dave: I can look at the backports to stable if needed.
include/net/tcp.h | 5 +++++
net/ipv6/icmp.c | 5 +++--
net/ipv6/tcp_ipv6.c | 6 ++++--
3 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 3482d13d655b..b073dca8f3ac 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -839,6 +839,11 @@ static inline void bpf_compute_data_end_sk_skb(struct sk_buff *skb)
*/
static inline int tcp_v6_iif(const struct sk_buff *skb)
{
+ return TCP_SKB_CB(skb)->header.h6.iif;
+}
+
+static inline int tcp_v6_iif_l3_slave(const struct sk_buff *skb)
+{
bool l3_slave = ipv6_l3mdev_skb(TCP_SKB_CB(skb)->header.h6.flags);
return l3_slave ? skb->skb_iif : TCP_SKB_CB(skb)->header.h6.iif;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index be491bf6ab6e..ef2505aefc15 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -402,9 +402,10 @@ static int icmp6_iif(const struct sk_buff *skb)
/* for local traffic to local address, skb dev is the loopback
* device. Check if there is a dst attached to the skb and if so
- * get the real device index.
+ * get the real device index. Same is needed for replies to a link
+ * local address on a device enslaved to an L3 master device
*/
- if (unlikely(iif == LOOPBACK_IFINDEX)) {
+ if (unlikely(iif == LOOPBACK_IFINDEX || netif_is_l3_master(skb->dev))) {
const struct rt6_info *rt6 = skb_rt6_info(skb);
if (rt6)
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 7efa9fd7e109..03e6b7a2bc53 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -938,7 +938,8 @@ static void tcp_v6_send_reset(const struct sock *sk, struct sk_buff *skb)
&tcp_hashinfo, NULL, 0,
&ipv6h->saddr,
th->source, &ipv6h->daddr,
- ntohs(th->source), tcp_v6_iif(skb),
+ ntohs(th->source),
+ tcp_v6_iif_l3_slave(skb),
tcp_v6_sdif(skb));
if (!sk1)
goto out;
@@ -1609,7 +1610,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
skb, __tcp_hdrlen(th),
&ipv6_hdr(skb)->saddr, th->source,
&ipv6_hdr(skb)->daddr,
- ntohs(th->dest), tcp_v6_iif(skb),
+ ntohs(th->dest),
+ tcp_v6_iif_l3_slave(skb),
sdif);
if (sk2) {
struct inet_timewait_sock *tw = inet_twsk(sk);
--
2.11.0
^ permalink raw reply related
* [PATCH] /net/9p/trans_fd.c: fix race-condition by flushing workqueue before the kfree()
From: Tomas Bortoli @ 2018-07-19 20:06 UTC (permalink / raw)
To: ericvh, rminnich, lucho
Cc: davem, v9fs-developer, netdev, linux-kernel, syzkaller,
Tomas Bortoli
The patch adds the flush in p9_mux_poll_stop() as it the function used by
p9_conn_destroy(), in turn called by p9_fd_close() to stop the async
polling associated with the data regarding the connection.
Signed-off-by: Tomas Bortoli <tomasbortoli@gmail.com>
Reported-by: syzbot+39749ed7d9ef6dfb23f6@syzkaller.appspotmail.com
---
As shown by Syzbot, it is possible to provoke a race between p9_fd_close()
and p9_poll_workfn() that is called to take care of the async read/write work
to do. To make sure p9_fd_close() frees "trans" when it is not used anymore,
it has to explicitly call flush_scheduled_work() before the kfree().
net/9p/trans_fd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/9p/trans_fd.c b/net/9p/trans_fd.c
index bf459ee0feab..76ae134c05d9 100644
--- a/net/9p/trans_fd.c
+++ b/net/9p/trans_fd.c
@@ -185,6 +185,8 @@ static void p9_mux_poll_stop(struct p9_conn *m)
spin_lock_irqsave(&p9_poll_lock, flags);
list_del_init(&m->poll_pending_link);
spin_unlock_irqrestore(&p9_poll_lock, flags);
+
+ flush_scheduled_work();
}
/**
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next 3/4] net/tc: introduce TC_ACT_MIRRED.
From: Jiri Pirko @ 2018-07-19 18:56 UTC (permalink / raw)
To: Paolo Abeni
Cc: netdev, Jamal Hadi Salim, Cong Wang, Daniel Borkmann,
Marcelo Ricardo Leitner, Eyal Birger
In-Reply-To: <78e96c4fd40645bb4bce45b6502ed34cf502356f.1531941678.git.pabeni@redhat.com>
Thu, Jul 19, 2018 at 03:02:28PM CEST, pabeni@redhat.com wrote:
>This is similar TC_ACT_REDIRECT, but with a slightly different
>semantic:
>- on ingress the mirred skbs are passed to the target device
>network stack without any additional check not scrubbing.
>- the rcu-protected stats provided via the tcf_result struct
> are updated on error conditions.
>
>v1 -> v2: do not touch TC_ACT_REDIRECT code path, introduce
> a new action type instead
>
>Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>---
> include/net/sch_generic.h | 19 +++++++++++++++++++
> include/uapi/linux/pkt_cls.h | 3 ++-
> net/core/dev.c | 4 ++++
> net/sched/act_api.c | 6 ++++--
> 4 files changed, 29 insertions(+), 3 deletions(-)
>
>diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>index 056dc1083aa3..667d7b66fee2 100644
>--- a/include/net/sch_generic.h
>+++ b/include/net/sch_generic.h
>@@ -235,6 +235,12 @@ struct tcf_result {
> u32 classid;
> };
> const struct tcf_proto *goto_tp;
>+
>+ /* used by the TC_ACT_MIRRED action */
>+ struct {
>+ bool ingress;
>+ struct gnet_stats_queue *qstats;
>+ };
> };
> };
>
>@@ -1091,4 +1097,17 @@ void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
> void mini_qdisc_pair_init(struct mini_Qdisc_pair *miniqp, struct Qdisc *qdisc,
> struct mini_Qdisc __rcu **p_miniq);
>
>+static inline void skb_tc_redirect(struct sk_buff *skb, struct tcf_result *res)
>+{
>+ struct gnet_stats_queue *stats = res->qstats;
>+ int ret;
>+
>+ if (res->ingress)
>+ ret = netif_receive_skb(skb);
>+ else
>+ ret = dev_queue_xmit(skb);
>+ if (ret && stats)
>+ qstats_overlimit_inc(res->qstats);
>+}
>+
> #endif
>diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
>index 7cdd62b51106..1a5e8a3217f3 100644
>--- a/include/uapi/linux/pkt_cls.h
>+++ b/include/uapi/linux/pkt_cls.h
>@@ -45,7 +45,8 @@ enum {
> * the skb and act like everything
> * is alright.
> */
>-#define TC_ACT_LAST TC_ACT_TRAP
>+#define TC_ACT_MIRRED 9
>+#define TC_ACT_LAST TC_ACT_MIRRED
>
> /* There is a special kind of actions called "extended actions",
> * which need a value parameter. These have a local opcode located in
>diff --git a/net/core/dev.c b/net/core/dev.c
>index 14a748ee8cc9..3822f29d730f 100644
>--- a/net/core/dev.c
>+++ b/net/core/dev.c
>@@ -4602,6 +4602,10 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
> __skb_push(skb, skb->mac_len);
> skb_do_redirect(skb);
> return NULL;
>+ case TC_ACT_MIRRED:
>+ /* this does not scrub the packet, and updates stats on error */
>+ skb_tc_redirect(skb, &cl_res);
REDIRECT does skb_do_redirect and MIRRED does skb_tc_redirect.
Confusing. I agree with Cong that the name is not correct.
>+ return NULL;
> default:
> break;
> }
>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>index f6438f246dab..029302e2813e 100644
>--- a/net/sched/act_api.c
>+++ b/net/sched/act_api.c
>@@ -895,8 +895,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> }
> }
>
>- if (a->tcfa_action == TC_ACT_REDIRECT) {
>- net_warn_ratelimited("TC_ACT_REDIRECT can't be used directly");
>+ if (a->tcfa_action == TC_ACT_REDIRECT ||
>+ a->tcfa_action == TC_ACT_MIRRED) {
>+ net_warn_ratelimited("action %d can't be used directly",
>+ a->tcfa_action);
> a->tcfa_action = TC_ACT_LAST + 1;
> }
>
>--
>2.17.1
>
^ permalink raw reply
* Re: [PATCH mlx5-next] RDMA/mlx5: Don't use cached IRQ affinity mask
From: Steve Wise @ 2018-07-19 18:45 UTC (permalink / raw)
To: Max Gurtovoy, 'Sagi Grimberg', 'Leon Romanovsky'
Cc: 'Doug Ledford', 'Jason Gunthorpe',
'RDMA mailing list', 'Saeed Mahameed',
'linux-netdev'
In-Reply-To: <c5362a12-4be6-9f17-7f06-a52aed69e3b6@mellanox.com>
On 7/19/2018 9:50 AM, Max Gurtovoy wrote:
>
>
> On 7/18/2018 10:29 PM, Steve Wise wrote:
>>
>>>
>>> On 7/18/2018 2:38 PM, Sagi Grimberg wrote:
>>>>
>>>>>> IMO we must fulfil the user wish to connect to N queues and not
>>>>>> reduce
>>>>>> it because of affinity overlaps. So in order to push Leon's patch we
>>>>>> must also fix the blk_mq_rdma_map_queues to do a best effort
>>> mapping
>>>>>> according the affinity and map the rest in naive way (in that way we
>>>>>> will *always* map all the queues).
>>>>>
>>>>> That is what I would expect also. For example, in my node, where
>>>>> there are
>>>>> 16 cpus, and 2 numa nodes, I observe much better nvmf IOPS
>>> performance by
>>>>> setting up my 16 driver completion event queues such that each is
>>>>> bound to a
>>>>> node-local cpu. So I end up with each nodel-local cpu having 2
>>>>> queues
>>>>> bound
>>>>> to it. W/O adding support in iw_cxgb4 for ib_get_vector_affinity(),
>>>>> this
>>>>> works fine. I assumed adding ib_get_vector_affinity() would allow
>>>>> this to
>>>>> all "just work" by default, but I'm running into this connection
>>>>> failure
>>>>> issue.
>>>>>
>>>>> I don't understand exactly what the blk_mq layer is trying to do,
>>>>> but I
>>>>> assume it has ingress event queues and processing that it trying
>>>>> to align
>>>>> with the drivers ingress cq event handling, so everybody stays on the
>>>>> same
>>>>> cpu (or at least node). But something else is going on. Is there
>>>>> documentation on how this works somewhere?
>>>>
>>>> Does this (untested) patch help?
>>>
>>> I'm not sure (I'll test it tomorrow) because the issue is the unmapped
>>> queues and not the cpus.
>>> for example, if the affinity of q=6 and q=12 returned the same cpumask
>>> than q=6 will not be mapped and will fail to connect.
>>>
>>
>> Attached is a patch that applies cleanly for me. It has problems if
>> vectors have affinity to more than 1 cpu:
>>
>> [ 2031.988881] iw_cxgb4: comp_vector 0, irq 203 mask 0xff00
>> [ 2031.994706] iw_cxgb4: comp_vector 1, irq 204 mask 0xff00
>> [ 2032.000348] iw_cxgb4: comp_vector 2, irq 205 mask 0xff00
>> [ 2032.005992] iw_cxgb4: comp_vector 3, irq 206 mask 0xff00
>> [ 2032.011629] iw_cxgb4: comp_vector 4, irq 207 mask 0xff00
>> [ 2032.017271] iw_cxgb4: comp_vector 5, irq 208 mask 0xff00
>> [ 2032.022901] iw_cxgb4: comp_vector 6, irq 209 mask 0xff00
>> [ 2032.028514] iw_cxgb4: comp_vector 7, irq 210 mask 0xff00
>> [ 2032.034110] iw_cxgb4: comp_vector 8, irq 211 mask 0xff00
>> [ 2032.039677] iw_cxgb4: comp_vector 9, irq 212 mask 0xff00
>> [ 2032.045244] iw_cxgb4: comp_vector 10, irq 213 mask 0xff00
>> [ 2032.050889] iw_cxgb4: comp_vector 11, irq 214 mask 0xff00
>> [ 2032.056531] iw_cxgb4: comp_vector 12, irq 215 mask 0xff00
>> [ 2032.062174] iw_cxgb4: comp_vector 13, irq 216 mask 0xff00
>> [ 2032.067817] iw_cxgb4: comp_vector 14, irq 217 mask 0xff00
>> [ 2032.073457] iw_cxgb4: comp_vector 15, irq 218 mask 0xff00
>> [ 2032.079102] blk_mq_rdma_map_queues: set->mq_map[0] queue 0 vector 0
>> [ 2032.085621] blk_mq_rdma_map_queues: set->mq_map[1] queue 1 vector 1
>> [ 2032.092139] blk_mq_rdma_map_queues: set->mq_map[2] queue 2 vector 2
>> [ 2032.098658] blk_mq_rdma_map_queues: set->mq_map[3] queue 3 vector 3
>> [ 2032.105177] blk_mq_rdma_map_queues: set->mq_map[4] queue 4 vector 4
>> [ 2032.111689] blk_mq_rdma_map_queues: set->mq_map[5] queue 5 vector 5
>> [ 2032.118208] blk_mq_rdma_map_queues: set->mq_map[6] queue 6 vector 6
>> [ 2032.124728] blk_mq_rdma_map_queues: set->mq_map[7] queue 7 vector 7
>> [ 2032.131246] blk_mq_rdma_map_queues: set->mq_map[8] queue 15 vector 15
>> [ 2032.137938] blk_mq_rdma_map_queues: set->mq_map[9] queue 15 vector 15
>> [ 2032.144629] blk_mq_rdma_map_queues: set->mq_map[10] queue 15
>> vector 15
>> [ 2032.151401] blk_mq_rdma_map_queues: set->mq_map[11] queue 15
>> vector 15
>> [ 2032.158172] blk_mq_rdma_map_queues: set->mq_map[12] queue 15
>> vector 15
>> [ 2032.164940] blk_mq_rdma_map_queues: set->mq_map[13] queue 15
>> vector 15
>> [ 2032.171709] blk_mq_rdma_map_queues: set->mq_map[14] queue 15
>> vector 15
>> [ 2032.178477] blk_mq_rdma_map_queues: set->mq_map[15] queue 15
>> vector 15
>> [ 2032.187409] nvme nvme0: Connect command failed, error wo/DNR bit:
>> -16402
>> [ 2032.194376] nvme nvme0: failed to connect queue: 9 ret=-18
>
> queue 9 is not mapped (overlap).
> please try the bellow:
>
This seems to work. Here are three mapping cases: each vector on its
own cpu, each vector on 1 cpu within the local numa node, and each
vector having all cpus in its numa node. The 2nd mapping looks kinda
funny, but I think it achieved what you wanted? And all the cases
resulted in successful connections.
#### each vector on its own cpu:
[ 3844.756229] iw_cxgb4: comp_vector 0, irq 203 mask 0x100
[ 3844.762104] iw_cxgb4: comp_vector 1, irq 204 mask 0x200
[ 3844.767896] iw_cxgb4: comp_vector 2, irq 205 mask 0x400
[ 3844.773663] iw_cxgb4: comp_vector 3, irq 206 mask 0x800
[ 3844.779405] iw_cxgb4: comp_vector 4, irq 207 mask 0x1000
[ 3844.785231] iw_cxgb4: comp_vector 5, irq 208 mask 0x2000
[ 3844.791043] iw_cxgb4: comp_vector 6, irq 209 mask 0x4000
[ 3844.796835] iw_cxgb4: comp_vector 7, irq 210 mask 0x8000
[ 3844.802619] iw_cxgb4: comp_vector 8, irq 211 mask 0x1
[ 3844.808133] iw_cxgb4: comp_vector 9, irq 212 mask 0x2
[ 3844.813643] iw_cxgb4: comp_vector 10, irq 213 mask 0x4
[ 3844.819235] iw_cxgb4: comp_vector 11, irq 214 mask 0x8
[ 3844.824817] iw_cxgb4: comp_vector 12, irq 215 mask 0x10
[ 3844.830486] iw_cxgb4: comp_vector 13, irq 216 mask 0x20
[ 3844.836148] iw_cxgb4: comp_vector 14, irq 217 mask 0x40
[ 3844.841804] iw_cxgb4: comp_vector 15, irq 218 mask 0x80
[ 3844.847456] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8
[ 3844.847457] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9
[ 3844.847457] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10
[ 3844.847458] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11
[ 3844.847459] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12
[ 3844.847459] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13
[ 3844.847460] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14
[ 3844.847461] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15
[ 3844.847462] blk_mq_rdma_map_queues: set->mq_map[8] queue 0 vector 0
[ 3844.847462] blk_mq_rdma_map_queues: set->mq_map[9] queue 1 vector 1
[ 3844.847463] blk_mq_rdma_map_queues: set->mq_map[10] queue 2 vector 2
[ 3844.847463] blk_mq_rdma_map_queues: set->mq_map[11] queue 3 vector 3
[ 3844.847464] blk_mq_rdma_map_queues: set->mq_map[12] queue 4 vector 4
[ 3844.847465] blk_mq_rdma_map_queues: set->mq_map[13] queue 5 vector 5
[ 3844.847465] blk_mq_rdma_map_queues: set->mq_map[14] queue 6 vector 6
[ 3844.847466] blk_mq_rdma_map_queues: set->mq_map[15] queue 7 vector 7
#### each vector on 1 cpu in is numa node
[ 3932.840244] iw_cxgb4: comp_vector 0, irq 203 mask 0x400
[ 3932.846018] iw_cxgb4: comp_vector 1, irq 204 mask 0x800
[ 3932.851687] iw_cxgb4: comp_vector 2, irq 205 mask 0x1000
[ 3932.857428] iw_cxgb4: comp_vector 3, irq 206 mask 0x2000
[ 3932.863160] iw_cxgb4: comp_vector 4, irq 207 mask 0x4000
[ 3932.868882] iw_cxgb4: comp_vector 5, irq 208 mask 0x8000
[ 3932.874594] iw_cxgb4: comp_vector 6, irq 209 mask 0x100
[ 3932.880213] iw_cxgb4: comp_vector 7, irq 210 mask 0x200
[ 3932.885831] iw_cxgb4: comp_vector 8, irq 211 mask 0x400
[ 3932.891440] iw_cxgb4: comp_vector 9, irq 212 mask 0x800
[ 3932.897043] iw_cxgb4: comp_vector 10, irq 213 mask 0x1000
[ 3932.902812] iw_cxgb4: comp_vector 11, irq 214 mask 0x2000
[ 3932.908580] iw_cxgb4: comp_vector 12, irq 215 mask 0x4000
[ 3932.914338] iw_cxgb4: comp_vector 13, irq 216 mask 0x8000
[ 3932.920096] iw_cxgb4: comp_vector 14, irq 217 mask 0x100
[ 3932.925756] iw_cxgb4: comp_vector 15, irq 218 mask 0x200
[ 3932.931413] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8
[ 3932.931414] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9
[ 3932.931415] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10
[ 3932.931416] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11
[ 3932.931416] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12
[ 3932.931417] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13
[ 3932.931418] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14
[ 3932.931418] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15
[ 3932.931419] blk_mq_rdma_map_queues: set->mq_map[8] queue 6 vector 6
[ 3932.931420] blk_mq_rdma_map_queues: set->mq_map[9] queue 7 vector 7
[ 3932.931420] blk_mq_rdma_map_queues: set->mq_map[10] queue 0 vector 0
[ 3932.931421] blk_mq_rdma_map_queues: set->mq_map[11] queue 1 vector 1
[ 3932.931422] blk_mq_rdma_map_queues: set->mq_map[12] queue 2 vector 2
[ 3932.931423] blk_mq_rdma_map_queues: set->mq_map[13] queue 3 vector 3
[ 3932.931423] blk_mq_rdma_map_queues: set->mq_map[14] queue 4 vector 4
[ 3932.931425] blk_mq_rdma_map_queues: set->mq_map[15] queue 5 vector 5
### each vector having all cpus in its numa node
[ 4047.308234] iw_cxgb4: comp_vector 0, irq 203 mask 0xff00
[ 4047.314042] iw_cxgb4: comp_vector 1, irq 204 mask 0xff00
[ 4047.319745] iw_cxgb4: comp_vector 2, irq 205 mask 0xff00
[ 4047.325417] iw_cxgb4: comp_vector 3, irq 206 mask 0xff00
[ 4047.331062] iw_cxgb4: comp_vector 4, irq 207 mask 0xff00
[ 4047.336703] iw_cxgb4: comp_vector 5, irq 208 mask 0xff00
[ 4047.342329] iw_cxgb4: comp_vector 6, irq 209 mask 0xff00
[ 4047.347953] iw_cxgb4: comp_vector 7, irq 210 mask 0xff00
[ 4047.353578] iw_cxgb4: comp_vector 8, irq 211 mask 0xff00
[ 4047.359204] iw_cxgb4: comp_vector 9, irq 212 mask 0xff00
[ 4047.364829] iw_cxgb4: comp_vector 10, irq 213 mask 0xff00
[ 4047.370544] iw_cxgb4: comp_vector 11, irq 214 mask 0xff00
[ 4047.376264] iw_cxgb4: comp_vector 12, irq 215 mask 0xff00
[ 4047.381983] iw_cxgb4: comp_vector 13, irq 216 mask 0xff00
[ 4047.387695] iw_cxgb4: comp_vector 14, irq 217 mask 0xff00
[ 4047.393406] iw_cxgb4: comp_vector 15, irq 218 mask 0xff00
[ 4047.399118] blk_mq_rdma_map_queues: set->mq_map[0] queue 8 vector 8
[ 4047.399119] blk_mq_rdma_map_queues: set->mq_map[1] queue 9 vector 9
[ 4047.399120] blk_mq_rdma_map_queues: set->mq_map[2] queue 10 vector 10
[ 4047.399121] blk_mq_rdma_map_queues: set->mq_map[3] queue 11 vector 11
[ 4047.399121] blk_mq_rdma_map_queues: set->mq_map[4] queue 12 vector 12
[ 4047.399122] blk_mq_rdma_map_queues: set->mq_map[5] queue 13 vector 13
[ 4047.399123] blk_mq_rdma_map_queues: set->mq_map[6] queue 14 vector 14
[ 4047.399123] blk_mq_rdma_map_queues: set->mq_map[7] queue 15 vector 15
[ 4047.399124] blk_mq_rdma_map_queues: set->mq_map[8] queue 0 vector 0
[ 4047.399125] blk_mq_rdma_map_queues: set->mq_map[9] queue 1 vector 1
[ 4047.399125] blk_mq_rdma_map_queues: set->mq_map[10] queue 2 vector 2
[ 4047.399126] blk_mq_rdma_map_queues: set->mq_map[11] queue 3 vector 3
[ 4047.399127] blk_mq_rdma_map_queues: set->mq_map[12] queue 4 vector 4
[ 4047.399127] blk_mq_rdma_map_queues: set->mq_map[13] queue 5 vector 5
[ 4047.399128] blk_mq_rdma_map_queues: set->mq_map[14] queue 6 vector 6
[ 4047.399128] blk_mq_rdma_map_queues: set->mq_map[15] queue 7 vector 7
^ permalink raw reply
* Re: [PATCH v4 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-19 18:42 UTC (permalink / raw)
To: Nishanth Devarajan
Cc: Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, Cody Doucette, Michel Machado
In-Reply-To: <20180719182243.GA4812@gmail.com>
On Thu, Jul 19, 2018 at 11:23 AM Nishanth Devarajan <ndev2021@gmail.com> wrote:
> +static int skbprio_change(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack)
> +{
> + struct skbprio_sched_data *q = qdisc_priv(sch);
> + struct tc_skbprio_qopt *ctl = nla_data(opt);
> + const unsigned int min_limit = 1;
> +
> + if (ctl->limit == (typeof(ctl->limit))-1)
> + sch->limit = max(qdisc_dev(sch)->tx_queue_len, min_limit);
I am sorry, I still don't like this use of tx_queue_len, it either
should be removed or just align to other existing use cases.
Apologize for responding to your previous email so late.
^ permalink raw reply
* Re: [PATCH v3 net-next] net/sched: add skbprio scheduler
From: Cong Wang @ 2018-07-19 18:39 UTC (permalink / raw)
To: Michel Machado
Cc: Nishanth Devarajan, Jamal Hadi Salim, Jiri Pirko, David Miller,
Linux Kernel Network Developers, Cody Doucette
In-Reply-To: <9f38f8c4-c2db-8b5d-1fea-4cb33a5838f3@digirati.com.br>
(Sorry for missing this email, it is lost in other discussions.)
On Wed, Jul 11, 2018 at 8:25 AM Michel Machado <michel@digirati.com.br> wrote:
>
> On 07/10/2018 10:57 PM, Cong Wang wrote:
> > The dev->tx_queue_len is fundamentally non-sense since now
> > almost every real NIC is multi-queue and qdisc has a completely
> > different sch->limit. This is why I suggested you to simply
> > avoid it in your code.
>
> Would you be okay with a constant there? If so, we could just put 64
> there. The optimal number is hardware dependent, but we don't know how
> to calculate it.
Yes, sure, fq_codel uses 10240 already. :)
>
> > There is no standard way to use dev->tx_queue_len in kernel,
> > so I can't claim your use is correct or not, but it still looks odd,
> > other qdisc seems just uses as a default, rather than picking
> > the smaller or bigger value as a cap.
>
> The reason for the `max(qdisc_dev(sch)->tx_queue_len, min_limit)` is
> to make sure that sch->limit is at least 1. We couldn't come up with a
> meaningful behavior for sch->limit being zero, so we defined the basis
> case of skbprio_enqueue() as sch->limit one. If there's a guarantee that
> qdisc_dev(sch)->tx_queue_len is always greater than zero, we don't need
> the max().
I think tx_queue_len could be 0. But again, why do you need to care
about tx_queue_len being 0 or not here?
sch->limit could be 0 too, it means this qdisc should not queue any
packets.
Thanks
^ permalink raw reply
* Re: [PATCH v3 1/2] tools/bpftool: ignore build products
From: Jakub Kicinski @ 2018-07-19 18:23 UTC (permalink / raw)
To: Taeung Song; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, linux-kernel
In-Reply-To: <20180719121005.12018-1-treeze.taeung@gmail.com>
On Thu, 19 Jul 2018 21:10:04 +0900, Taeung Song wrote:
> For untracked things of tools/bpf, add this.
>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
> tools/bpf/.gitignore | 5 +++++
> tools/bpf/bpftool/.gitignore | 1 +
> 2 files changed, 6 insertions(+)
> create mode 100644 tools/bpf/.gitignore
>
> diff --git a/tools/bpf/.gitignore b/tools/bpf/.gitignore
> new file mode 100644
> index 000000000000..dfe2bd5a4b95
> --- /dev/null
> +++ b/tools/bpf/.gitignore
> @@ -0,0 +1,5 @@
> +FEATURE-DUMP.bpf
> +bpf_asm
> +bpf_dbg
> +bpf_exp.yacc.*
> +bpf_jit_disasm
> diff --git a/tools/bpf/bpftool/.gitignore b/tools/bpf/bpftool/.gitignore
> index d7e678c2d396..103cc5b5b446 100644
> --- a/tools/bpf/bpftool/.gitignore
> +++ b/tools/bpf/bpftool/.gitignore
> @@ -1,3 +1,4 @@
> *.d
> bpftool
> +bpftool*.8
> FEATURE-DUMP.bpftool
This patch is going to be merged to the bpf-next tree, I presume, and
there is more man pages there:
$ make -C tools/bpf/bpftool/ doc
make: Entering directory 'linux/tools/bpf/bpftool'
DESCEND Documentation
make[1]: Entering directory '/linux/tools/bpf/bpftool/Documentation'
GEN bpftool-perf.8
GEN bpftool-map.8
GEN bpftool.8
GEN bpftool-prog.8
GEN bpftool-cgroup.8
GEN bpf-helpers.rst
Parsed description of 80 helper function(s)
GEN bpf-helpers.7
make[1]: Leaving directory 'linux/tools/bpf/bpftool/Documentation'
make: Leaving directory 'linux/tools/bpf/bpftool'
$ git status
On branch work
Your branch is ahead of 'pending' by 10 commits.
(use "git push" to publish your local commits)
Untracked files:
(use "git add <file>..." to include in what will be committed)
tools/bpf/bpftool/Documentation/bpf-helpers.7
tools/bpf/bpftool/Documentation/bpf-helpers.rst
tools/bpf/bpftool/Documentation/bpftool-cgroup.8
tools/bpf/bpftool/Documentation/bpftool-map.8
tools/bpf/bpftool/Documentation/bpftool-perf.8
tools/bpf/bpftool/Documentation/bpftool-prog.8
tools/bpf/bpftool/Documentation/bpftool.8
nothing added to commit but untracked files present (use "git add" to track)
See the bpf-helpers.* files? Those are all auto-generated.
If you respin for -next please split the series and put [PATCH bpf-next
v4] as prefix of first patch, and [PATCH bpf v4] as prefix for the
second, as one of them is an improvement, and second a bug fix.
Thank you!!
^ permalink raw reply
* [PATCH v4 net-next] net/sched: add skbprio scheduler
From: Nishanth Devarajan @ 2018-07-19 18:22 UTC (permalink / raw)
To: xiyou.wangcong, jhs, jiri, davem; +Cc: netdev, doucette, michel
net/sched: add skbprio scheduer
Skbprio (SKB Priority Queue) is a queueing discipline that prioritizes packets
according to their skb->priority field. Under congestion, already-enqueued lower
priority packets will be dropped to make space available for higher priority
packets. Skbprio was conceived as a solution for denial-of-service defenses that
need to route packets with different priorities as a means to overcome DoS
attacks.
v4
*Drop Documentation/networking/sch_skbprio.txt doc file to move it to tc man
page for Skbprio, in iproute2.
v3
*Drop max_limit parameter in struct skbprio_sched_data and instead use
sch->limit.
*Reference qdisc_dev(sch)->tx_queue_len only once, during initialisation for
qdisc (previously being referenced every time qdisc changes).
*Move qdisc's detailed description from in-code to Documentation/networking.
*When qdisc is saturated, enqueue incoming packet first before dequeueing
lowest priority packet in queue - improves usage of call stack registers.
*Introduce and use overlimit stat to keep track of number of dropped packets.
v2
*Use skb->priority field rather than DS field. Rename queueing discipline as
SKB Priority Queue (previously Gatekeeper Priority Queue).
*Queueing discipline is made classful to expose Skbprio's internal priority
queues.
Signed-off-by: Nishanth Devarajan <ndev2021@gmail.com>
Reviewed-by: Sachin Paryani <sachin.paryani@gmail.com>
Reviewed-by: Cody Doucette <doucette@bu.edu>
Reviewed-by: Michel Machado <michel@digirati.com.br>
---
include/uapi/linux/pkt_sched.h | 15 ++
net/sched/Kconfig | 13 ++
net/sched/Makefile | 1 +
net/sched/sch_skbprio.c | 330 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 359 insertions(+)
create mode 100644 net/sched/sch_skbprio.c
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index d9cc9dc..8975fd1 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -124,6 +124,21 @@ struct tc_fifo_qopt {
__u32 limit; /* Queue length: bytes for bfifo, packets for pfifo */
};
+/* SKBPRIO section */
+
+/*
+ * Priorities go from zero to (SKBPRIO_MAX_PRIORITY - 1).
+ * SKBPRIO_MAX_PRIORITY should be at least 64 in order for skbprio to be able
+ * to map one to one the DS field of IPV4 and IPV6 headers.
+ * Memory allocation grows linearly with SKBPRIO_MAX_PRIORITY.
+ */
+
+#define SKBPRIO_MAX_PRIORITY 64
+
+struct tc_skbprio_qopt {
+ __u32 limit; /* Queue length in packets. */
+};
+
/* PRIO section */
#define TCQ_PRIO_BANDS 16
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index 7af2467..7699344 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -251,6 +251,19 @@ config NET_SCH_MQPRIO
If unsure, say N.
+config NET_SCH_SKBPRIO
+ tristate "SKB priority queue scheduler (SKBPRIO)"
+ help
+ Say Y here if you want to use the SKB priority queue
+ scheduler. This schedules packets according to skb->priority,
+ which is useful for request packets in DoS mitigation systems such
+ as Gatekeeper.
+
+ To compile this driver as a module, choose M here: the module will
+ be called sch_skbprio.
+
+ If unsure, say N.
+
config NET_SCH_CHOKE
tristate "CHOose and Keep responsive flow scheduler (CHOKE)"
help
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 673ee7d..112ef70 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -46,6 +46,7 @@ obj-$(CONFIG_NET_SCH_NETEM) += sch_netem.o
obj-$(CONFIG_NET_SCH_DRR) += sch_drr.o
obj-$(CONFIG_NET_SCH_PLUG) += sch_plug.o
obj-$(CONFIG_NET_SCH_MQPRIO) += sch_mqprio.o
+obj-$(CONFIG_NET_SCH_SKBPRIO) += sch_skbprio.o
obj-$(CONFIG_NET_SCH_CHOKE) += sch_choke.o
obj-$(CONFIG_NET_SCH_QFQ) += sch_qfq.o
obj-$(CONFIG_NET_SCH_CODEL) += sch_codel.o
diff --git a/net/sched/sch_skbprio.c b/net/sched/sch_skbprio.c
new file mode 100644
index 0000000..6b94f54
--- /dev/null
+++ b/net/sched/sch_skbprio.c
@@ -0,0 +1,330 @@
+/*
+ * net/sched/sch_skbprio.c SKB Priority Queue.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Nishanth Devarajan, <ndev2021@gmail.com>
+ * Cody Doucette, <doucette@bu.edu>
+ * original idea by Michel Machado, Cody Doucette, and Qiaobin Fu
+ */
+
+#include <linux/string.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/pkt_sched.h>
+#include <net/sch_generic.h>
+#include <net/inet_ecn.h>
+
+/* SKB Priority Queue
+ * =================================
+ *
+ * Skbprio (SKB Priority Queue) is a queueing discipline that prioritizes
+ * packets according to their skb->priority field. Under congestion,
+ * Skbprio drops already-enqueued lower priority packets to make space
+ * available for higher priority packets; it was conceived as a solution
+ * for denial-of-service defenses that need to route packets with different
+ * priorities as a mean to overcome DoS attacks.
+ */
+
+struct skbprio_sched_data {
+ /* Queue state. */
+ struct sk_buff_head qdiscs[SKBPRIO_MAX_PRIORITY];
+ struct gnet_stats_queue qstats[SKBPRIO_MAX_PRIORITY];
+ u16 highest_prio;
+ u16 lowest_prio;
+};
+
+static u16 calc_new_high_prio(const struct skbprio_sched_data *q)
+{
+ int prio;
+
+ for (prio = q->highest_prio - 1; prio >= q->lowest_prio; prio--) {
+ if (!skb_queue_empty(&q->qdiscs[prio]))
+ return prio;
+ }
+
+ /* SKB queue is empty, return 0 (default highest priority setting). */
+ return 0;
+}
+
+static u16 calc_new_low_prio(const struct skbprio_sched_data *q)
+{
+ int prio;
+
+ for (prio = q->lowest_prio + 1; prio <= q->highest_prio; prio++) {
+ if (!skb_queue_empty(&q->qdiscs[prio]))
+ return prio;
+ }
+
+ /* SKB queue is empty, return SKBPRIO_MAX_PRIORITY - 1
+ * (default lowest priority setting).
+ */
+ return SKBPRIO_MAX_PRIORITY - 1;
+}
+
+static int skbprio_enqueue(struct sk_buff *skb, struct Qdisc *sch,
+ struct sk_buff **to_free)
+{
+ const unsigned int max_priority = SKBPRIO_MAX_PRIORITY - 1;
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct sk_buff_head *qdisc;
+ struct sk_buff_head *lp_qdisc;
+ struct sk_buff *to_drop;
+ u16 prio, lp;
+
+ /* Obtain the priority of @skb. */
+ prio = min(skb->priority, max_priority);
+
+ qdisc = &q->qdiscs[prio];
+ if (sch->q.qlen < sch->limit) {
+ __skb_queue_tail(qdisc, skb);
+ qdisc_qstats_backlog_inc(sch, skb);
+ q->qstats[prio].backlog += qdisc_pkt_len(skb);
+
+ /* Check to update highest and lowest priorities. */
+ if (prio > q->highest_prio)
+ q->highest_prio = prio;
+
+ if (prio < q->lowest_prio)
+ q->lowest_prio = prio;
+
+ sch->q.qlen++;
+ return NET_XMIT_SUCCESS;
+ }
+
+ /* If this packet has the lowest priority, drop it. */
+ lp = q->lowest_prio;
+ if (prio <= lp) {
+ q->qstats[prio].drops++;
+ q->qstats[prio].overlimits++;
+ return qdisc_drop(skb, sch, to_free);
+ }
+
+ __skb_queue_tail(qdisc, skb);
+ qdisc_qstats_backlog_inc(sch, skb);
+ q->qstats[prio].backlog += qdisc_pkt_len(skb);
+
+ /* Drop the packet at the tail of the lowest priority qdisc. */
+ lp_qdisc = &q->qdiscs[lp];
+ to_drop = __skb_dequeue_tail(lp_qdisc);
+ BUG_ON(!to_drop);
+ qdisc_qstats_backlog_dec(sch, to_drop);
+ qdisc_drop(to_drop, sch, to_free);
+
+ q->qstats[lp].backlog -= qdisc_pkt_len(to_drop);
+ q->qstats[lp].drops++;
+ q->qstats[lp].overlimits++;
+
+ /* Check to update highest and lowest priorities. */
+ if (skb_queue_empty(lp_qdisc)) {
+ if (q->lowest_prio == q->highest_prio) {
+ /* The incoming packet is the only packet in queue. */
+ BUG_ON(sch->q.qlen != 1);
+ q->lowest_prio = prio;
+ q->highest_prio = prio;
+ } else {
+ q->lowest_prio = calc_new_low_prio(q);
+ }
+ }
+
+ if (prio > q->highest_prio)
+ q->highest_prio = prio;
+
+ return NET_XMIT_CN;
+}
+
+static struct sk_buff *skbprio_dequeue(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct sk_buff_head *hpq = &q->qdiscs[q->highest_prio];
+ struct sk_buff *skb = __skb_dequeue(hpq);
+
+ if (unlikely(!skb))
+ return NULL;
+
+ sch->q.qlen--;
+ qdisc_qstats_backlog_dec(sch, skb);
+ qdisc_bstats_update(sch, skb);
+
+ q->qstats[q->highest_prio].backlog -= qdisc_pkt_len(skb);
+
+ /* Update highest priority field. */
+ if (skb_queue_empty(hpq)) {
+ if (q->lowest_prio == q->highest_prio) {
+ BUG_ON(sch->q.qlen);
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+ } else {
+ q->highest_prio = calc_new_high_prio(q);
+ }
+ }
+ return skb;
+}
+
+static int skbprio_change(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct tc_skbprio_qopt *ctl = nla_data(opt);
+ const unsigned int min_limit = 1;
+
+ if (ctl->limit == (typeof(ctl->limit))-1)
+ sch->limit = max(qdisc_dev(sch)->tx_queue_len, min_limit);
+ else if (ctl->limit < min_limit)
+ return -EINVAL;
+ else
+ sch->limit = ctl->limit;
+
+ return 0;
+}
+
+static int skbprio_init(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ const unsigned int min_limit = 1;
+ int prio;
+
+ /* Initialise all queues, one for each possible priority. */
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_head_init(&q->qdiscs[prio]);
+
+ memset(&q->qstats, 0, sizeof(q->qstats));
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+ if (!opt) {
+ sch->limit = max(qdisc_dev(sch)->tx_queue_len, min_limit);
+ return 0;
+ }
+ return skbprio_change(sch, opt, extack);
+}
+
+static int skbprio_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ struct tc_skbprio_qopt opt;
+
+ opt.limit = sch->limit;
+
+ if (nla_put(skb, TCA_OPTIONS, sizeof(opt), &opt))
+ return -1;
+
+ return skb->len;
+}
+
+static void skbprio_reset(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ int prio;
+
+ sch->qstats.backlog = 0;
+ sch->q.qlen = 0;
+
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_purge(&q->qdiscs[prio]);
+
+ memset(&q->qstats, 0, sizeof(q->qstats));
+ q->highest_prio = 0;
+ q->lowest_prio = SKBPRIO_MAX_PRIORITY - 1;
+}
+
+static void skbprio_destroy(struct Qdisc *sch)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ int prio;
+
+ for (prio = 0; prio < SKBPRIO_MAX_PRIORITY; prio++)
+ __skb_queue_purge(&q->qdiscs[prio]);
+}
+
+static struct Qdisc *skbprio_leaf(struct Qdisc *sch, unsigned long arg)
+{
+ return NULL;
+}
+
+static unsigned long skbprio_find(struct Qdisc *sch, u32 classid)
+{
+ return 0;
+}
+
+static int skbprio_dump_class(struct Qdisc *sch, unsigned long cl,
+ struct sk_buff *skb, struct tcmsg *tcm)
+{
+ tcm->tcm_handle |= TC_H_MIN(cl);
+ return 0;
+}
+
+static int skbprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
+ struct gnet_dump *d)
+{
+ struct skbprio_sched_data *q = qdisc_priv(sch);
+ if (gnet_stats_copy_queue(d, NULL, &q->qstats[cl - 1],
+ q->qstats[cl - 1].qlen) < 0)
+ return -1;
+ return 0;
+}
+
+static void skbprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+ unsigned int i;
+
+ if (arg->stop)
+ return;
+
+ for (i = 0; i < SKBPRIO_MAX_PRIORITY; i++) {
+ if (arg->count < arg->skip) {
+ arg->count++;
+ continue;
+ }
+ if (arg->fn(sch, i + 1, arg) < 0) {
+ arg->stop = 1;
+ break;
+ }
+ arg->count++;
+ }
+}
+
+static const struct Qdisc_class_ops skbprio_class_ops = {
+ .leaf = skbprio_leaf,
+ .find = skbprio_find,
+ .dump = skbprio_dump_class,
+ .dump_stats = skbprio_dump_class_stats,
+ .walk = skbprio_walk,
+};
+
+static struct Qdisc_ops skbprio_qdisc_ops __read_mostly = {
+ .cl_ops = &skbprio_class_ops,
+ .id = "skbprio",
+ .priv_size = sizeof(struct skbprio_sched_data),
+ .enqueue = skbprio_enqueue,
+ .dequeue = skbprio_dequeue,
+ .peek = qdisc_peek_dequeued,
+ .init = skbprio_init,
+ .reset = skbprio_reset,
+ .change = skbprio_change,
+ .dump = skbprio_dump,
+ .destroy = skbprio_destroy,
+ .owner = THIS_MODULE,
+};
+
+static int __init skbprio_module_init(void)
+{
+ return register_qdisc(&skbprio_qdisc_ops);
+}
+
+static void __exit skbprio_module_exit(void)
+{
+ unregister_qdisc(&skbprio_qdisc_ops);
+}
+
+module_init(skbprio_module_init)
+module_exit(skbprio_module_exit)
+
+MODULE_LICENSE("GPL");
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 3/4] net/tc: introduce TC_ACT_MIRRED.
From: Cong Wang @ 2018-07-19 18:07 UTC (permalink / raw)
To: Paolo Abeni
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
Daniel Borkmann, Marcelo Ricardo Leitner, Eyal Birger
In-Reply-To: <78e96c4fd40645bb4bce45b6502ed34cf502356f.1531941678.git.pabeni@redhat.com>
On Thu, Jul 19, 2018 at 6:03 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> This is similar TC_ACT_REDIRECT, but with a slightly different
> semantic:
> - on ingress the mirred skbs are passed to the target device
> network stack without any additional check not scrubbing.
> - the rcu-protected stats provided via the tcf_result struct
> are updated on error conditions.
At least its name sucks, it means to skip the skb_clone(),
that is avoid a copy, but you still call it MIRRED...
MIRRED means MIRror and REDirect.
Also, I don't understand why this new TC_ACT code needs
to be visible to user-space, whether to clone or not is purely
internal.
^ permalink raw reply
* Re: [PATCH net-next 4/4] act_mirred: use ACT_REDIRECT when possible
From: Cong Wang @ 2018-07-19 17:56 UTC (permalink / raw)
To: Paolo Abeni
Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner,
Eyal Birger
In-Reply-To: <a61342740ba2a4b3c6481d2133889e93dc788eef.camel@redhat.com>
On Wed, Jul 18, 2018 at 3:05 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> Hi,
>
> On Tue, 2018-07-17 at 10:24 -0700, Cong Wang wrote:
> > If you goal is to get rid of skb_clone(), why not just do the following?
> >
> > if (tcf_mirred_is_act_redirect(m_eaction)) {
> > skb2 = skb;
> > } else {
> > skb2 = skb_clone(skb, GFP_ATOMIC);
> > if (!skb2)
> > goto out;
> > }
> >
> > For redirect, we return TC_ACT_SHOT, so upper layer should not
> > touch the skb after that.
> >
> > What am I missing here?
>
> With ACT_SHOT caller/upper layer will free the skb, too. We will have
> an use after free (from either the upper layer and the xmit device).
> Similar issues with STOLEN, TRAP, etc.
>
> In the past, Changli Gao attempted to avoid the clone incrementing the
> skb usage count:
>
> commit 210d6de78c5d7c785fc532556cea340e517955e1
> Author: Changli Gao <xiaosuo@gmail.com>
> Date: Thu Jun 24 16:25:12 2010 +0000
>
> act_mirred: don't clone skb when skb isn't shared
>
> but some/many device drivers expect an skb usage count of 1, and that
> caused ooops and was revered.
Interesting, I wasn't aware of the above commit and its revert.
First, I didn't use skb_get() above.
Second, I think the caller of dev_queue_xmit() should not
touch the skb after it, the skb is either freed by dev_queue_xmit()
or successfully transmitted, in either case, the ownership belongs
to dev_queue_xmit(). So, I think we should skip the qdisc_drop()
for this case.
Not sure about netif_receive_skb() case, given veth calls in its
xmit too, I speculate the rule is probably same.
Not sure about other ACT_SHOT case than act_mirred...
>
> I think the only other option (beyond re-using ACT_MIRROR) is adding
> another action value, and let the upper layer re-inject the packet
> while handling such action (similar to what ACT_MIRROR currently does,
> but preserving the current mirred semantic).
Maybe if you mean to avoid breaking ACT_SHOT.
Thanks.
^ permalink raw reply
* [PATCH ipsec-next] xfrm: Remove xfrmi interface ID from flowi
From: Benedict Wong @ 2018-07-19 17:50 UTC (permalink / raw)
To: netdev; +Cc: nharold, lorenzo, Benedict Wong
In order to remove performance impact of having the extra u32 in every
single flowi, this change removes the flowi_xfrm struct, prefering to
take the if_id as a method parameter where needed.
In the inbound direction, if_id is only needed during the
__xfrm_check_policy() function, and the if_id can be determined at that
point based on the skb. As such, xfrmi_decode_session() is only called
with the skb in __xfrm_check_policy().
In the outbound direction, the only place where if_id is needed is the
xfrm_lookup() call in xfrmi_xmit2(). With this change, the if_id is
directly passed into the xfrm_lookup_with_ifid() call. All existing
callers can still call xfrm_lookup(), which uses a default if_id of 0.
This change does not change any behavior of XFRMIs except for improving
overall system performance via flowi size reduction.
This change has been tested against the Android Kernel Networking Tests:
https://android.googlesource.com/kernel/tests/+/master/net/test
Signed-off-by: Benedict Wong <benedictwong@google.com>
---
include/net/dst.h | 14 ++++++
include/net/flow.h | 9 ----
include/net/xfrm.h | 2 +-
net/xfrm/xfrm_interface.c | 4 +-
net/xfrm/xfrm_policy.c | 98 ++++++++++++++++++++++++++-------------
net/xfrm/xfrm_state.c | 3 +-
6 files changed, 83 insertions(+), 47 deletions(-)
diff --git a/include/net/dst.h b/include/net/dst.h
index b3219cd8a5a1..7f735e76ca73 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -475,6 +475,14 @@ static inline struct dst_entry *xfrm_lookup(struct net *net,
return dst_orig;
}
+static inline struct dst_entry *
+xfrm_lookup_with_ifid(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl, const struct sock *sk,
+ int flags, u32 if_id)
+{
+ return dst_orig;
+}
+
static inline struct dst_entry *xfrm_lookup_route(struct net *net,
struct dst_entry *dst_orig,
const struct flowi *fl,
@@ -494,6 +502,12 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
const struct flowi *fl, const struct sock *sk,
int flags);
+struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
+ struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ const struct sock *sk, int flags,
+ u32 if_id);
+
struct dst_entry *xfrm_lookup_route(struct net *net, struct dst_entry *dst_orig,
const struct flowi *fl, const struct sock *sk,
int flags);
diff --git a/include/net/flow.h b/include/net/flow.h
index 187c9bef672f..8ce21793094e 100644
--- a/include/net/flow.h
+++ b/include/net/flow.h
@@ -26,10 +26,6 @@ struct flowi_tunnel {
__be64 tun_id;
};
-struct flowi_xfrm {
- __u32 if_id;
-};
-
struct flowi_common {
int flowic_oif;
int flowic_iif;
@@ -43,7 +39,6 @@ struct flowi_common {
#define FLOWI_FLAG_SKIP_NH_OIF 0x04
__u32 flowic_secid;
struct flowi_tunnel flowic_tun_key;
- struct flowi_xfrm xfrm;
kuid_t flowic_uid;
};
@@ -83,7 +78,6 @@ struct flowi4 {
#define flowi4_secid __fl_common.flowic_secid
#define flowi4_tun_key __fl_common.flowic_tun_key
#define flowi4_uid __fl_common.flowic_uid
-#define flowi4_xfrm __fl_common.xfrm
/* (saddr,daddr) must be grouped, same order as in IP header */
__be32 saddr;
@@ -115,7 +109,6 @@ static inline void flowi4_init_output(struct flowi4 *fl4, int oif,
fl4->flowi4_flags = flags;
fl4->flowi4_secid = 0;
fl4->flowi4_tun_key.tun_id = 0;
- fl4->flowi4_xfrm.if_id = 0;
fl4->flowi4_uid = uid;
fl4->daddr = daddr;
fl4->saddr = saddr;
@@ -145,7 +138,6 @@ struct flowi6 {
#define flowi6_secid __fl_common.flowic_secid
#define flowi6_tun_key __fl_common.flowic_tun_key
#define flowi6_uid __fl_common.flowic_uid
-#define flowi6_xfrm __fl_common.xfrm
struct in6_addr daddr;
struct in6_addr saddr;
/* Note: flowi6_tos is encoded in flowlabel, too. */
@@ -193,7 +185,6 @@ struct flowi {
#define flowi_secid u.__fl_common.flowic_secid
#define flowi_tun_key u.__fl_common.flowic_tun_key
#define flowi_uid u.__fl_common.flowic_uid
-#define flowi_xfrm u.__fl_common.xfrm
} __attribute__((__aligned__(BITS_PER_LONG/8)));
static inline struct flowi *flowi4_to_flowi(struct flowi4 *fl4)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 1350e2cf0749..ca820945f30c 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1557,7 +1557,7 @@ struct xfrm_state *xfrm_state_find(const xfrm_address_t *daddr,
const struct flowi *fl,
struct xfrm_tmpl *tmpl,
struct xfrm_policy *pol, int *err,
- unsigned short family);
+ unsigned short family, u32 if_id);
struct xfrm_state *xfrm_stateonly_find(struct net *net, u32 mark, u32 if_id,
xfrm_address_t *daddr,
xfrm_address_t *saddr,
diff --git a/net/xfrm/xfrm_interface.c b/net/xfrm/xfrm_interface.c
index 31cb1c7e3881..ccfe18d67e98 100644
--- a/net/xfrm/xfrm_interface.c
+++ b/net/xfrm/xfrm_interface.c
@@ -307,10 +307,8 @@ xfrmi_xmit2(struct sk_buff *skb, struct net_device *dev, struct flowi *fl)
if (!dst)
goto tx_err_link_failure;
- fl->flowi_xfrm.if_id = xi->p.if_id;
-
dst_hold(dst);
- dst = xfrm_lookup(xi->net, dst, fl, NULL, 0);
+ dst = xfrm_lookup_with_ifid(xi->net, dst, fl, NULL, 0, xi->p.if_id);
if (IS_ERR(dst)) {
err = PTR_ERR(dst);
dst = NULL;
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 5d2f734f4309..2f70fe68b9b0 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1068,14 +1068,14 @@ EXPORT_SYMBOL(xfrm_policy_walk_done);
*/
static int xfrm_policy_match(const struct xfrm_policy *pol,
const struct flowi *fl,
- u8 type, u16 family, int dir)
+ u8 type, u16 family, int dir, u32 if_id)
{
const struct xfrm_selector *sel = &pol->selector;
int ret = -ESRCH;
bool match;
if (pol->family != family ||
- pol->if_id != fl->flowi_xfrm.if_id ||
+ pol->if_id != if_id ||
(fl->flowi_mark & pol->mark.m) != pol->mark.v ||
pol->type != type)
return ret;
@@ -1090,7 +1090,8 @@ static int xfrm_policy_match(const struct xfrm_policy *pol,
static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
const struct flowi *fl,
- u16 family, u8 dir)
+ u16 family, u8 dir,
+ u32 if_id)
{
int err;
struct xfrm_policy *pol, *ret;
@@ -1114,7 +1115,7 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
priority = ~0U;
ret = NULL;
hlist_for_each_entry_rcu(pol, chain, bydst) {
- err = xfrm_policy_match(pol, fl, type, family, dir);
+ err = xfrm_policy_match(pol, fl, type, family, dir, if_id);
if (err) {
if (err == -ESRCH)
continue;
@@ -1133,7 +1134,7 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
if ((pol->priority >= priority) && ret)
break;
- err = xfrm_policy_match(pol, fl, type, family, dir);
+ err = xfrm_policy_match(pol, fl, type, family, dir, if_id);
if (err) {
if (err == -ESRCH)
continue;
@@ -1158,21 +1159,25 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
return ret;
}
-static struct xfrm_policy *
-xfrm_policy_lookup(struct net *net, const struct flowi *fl, u16 family, u8 dir)
+static struct xfrm_policy *xfrm_policy_lookup(struct net *net,
+ const struct flowi *fl,
+ u16 family, u8 dir, u32 if_id)
{
#ifdef CONFIG_XFRM_SUB_POLICY
struct xfrm_policy *pol;
- pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_SUB, fl, family, dir);
+ pol = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_SUB, fl, family,
+ dir, if_id);
if (pol != NULL)
return pol;
#endif
- return xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN, fl, family, dir);
+ return xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN, fl, family,
+ dir, if_id);
}
static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir,
- const struct flowi *fl, u16 family)
+ const struct flowi *fl,
+ u16 family, u32 if_id)
{
struct xfrm_policy *pol;
@@ -1191,7 +1196,7 @@ static struct xfrm_policy *xfrm_sk_policy_lookup(const struct sock *sk, int dir,
match = xfrm_selector_match(&pol->selector, fl, family);
if (match) {
if ((sk->sk_mark & pol->mark.m) != pol->mark.v ||
- pol->if_id != fl->flowi_xfrm.if_id) {
+ pol->if_id != if_id) {
pol = NULL;
goto out;
}
@@ -1405,7 +1410,8 @@ xfrm_tmpl_resolve_one(struct xfrm_policy *policy, const struct flowi *fl,
}
}
- x = xfrm_state_find(remote, local, fl, tmpl, policy, &error, family);
+ x = xfrm_state_find(remote, local, fl, tmpl, policy, &error,
+ family, policy->if_id);
if (x && x->km.state == XFRM_STATE_VALID) {
xfrm[nx++] = x;
@@ -1708,7 +1714,8 @@ static int xfrm_expand_policies(const struct flowi *fl, u16 family,
pols[1] = xfrm_policy_lookup_bytype(xp_net(pols[0]),
XFRM_POLICY_TYPE_MAIN,
fl, family,
- XFRM_POLICY_OUT);
+ XFRM_POLICY_OUT,
+ pols[0]->if_id);
if (pols[1]) {
if (IS_ERR(pols[1])) {
xfrm_pols_put(pols, *num_pols);
@@ -1942,8 +1949,10 @@ static struct xfrm_dst *xfrm_create_dummy_bundle(struct net *net,
goto out;
}
-static struct xfrm_dst *
-xfrm_bundle_lookup(struct net *net, const struct flowi *fl, u16 family, u8 dir, struct xfrm_flo *xflo)
+static struct xfrm_dst *xfrm_bundle_lookup(struct net *net,
+ const struct flowi *fl,
+ u16 family, u8 dir,
+ struct xfrm_flo *xflo, u32 if_id)
{
struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
int num_pols = 0, num_xfrms = 0, err;
@@ -1952,7 +1961,7 @@ xfrm_bundle_lookup(struct net *net, const struct flowi *fl, u16 family, u8 dir,
/* Resolve policies to use if we couldn't get them from
* previous cache entry */
num_pols = 1;
- pols[0] = xfrm_policy_lookup(net, fl, family, dir);
+ pols[0] = xfrm_policy_lookup(net, fl, family, dir, if_id);
err = xfrm_expand_policies(fl, family, pols,
&num_pols, &num_xfrms);
if (err < 0)
@@ -2020,14 +2029,19 @@ static struct dst_entry *make_blackhole(struct net *net, u16 family,
return ret;
}
-/* Main function: finds/creates a bundle for given flow.
+/* Finds/creates a bundle for given flow and if_id
*
* At the moment we eat a raw IP route. Mostly to speed up lookups
* on interfaces with disabled IPsec.
+ *
+ * xfrm_lookup uses an if_id of 0 by default, and is provided for
+ * compatibility
*/
-struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
- const struct flowi *fl,
- const struct sock *sk, int flags)
+struct dst_entry *xfrm_lookup_with_ifid(struct net *net,
+ struct dst_entry *dst_orig,
+ const struct flowi *fl,
+ const struct sock *sk,
+ int flags, u32 if_id)
{
struct xfrm_policy *pols[XFRM_POLICY_TYPE_MAX];
struct xfrm_dst *xdst;
@@ -2043,7 +2057,8 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
sk = sk_const_to_full_sk(sk);
if (sk && sk->sk_policy[XFRM_POLICY_OUT]) {
num_pols = 1;
- pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl, family);
+ pols[0] = xfrm_sk_policy_lookup(sk, XFRM_POLICY_OUT, fl, family,
+ if_id);
err = xfrm_expand_policies(fl, family, pols,
&num_pols, &num_xfrms);
if (err < 0)
@@ -2087,7 +2102,7 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
!net->xfrm.policy_count[XFRM_POLICY_OUT])
goto nopol;
- xdst = xfrm_bundle_lookup(net, fl, family, dir, &xflo);
+ xdst = xfrm_bundle_lookup(net, fl, family, dir, &xflo, if_id);
if (xdst == NULL)
goto nopol;
if (IS_ERR(xdst)) {
@@ -2168,6 +2183,19 @@ struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
xfrm_pols_put(pols, drop_pols);
return ERR_PTR(err);
}
+EXPORT_SYMBOL(xfrm_lookup_with_ifid);
+
+/* Main function: finds/creates a bundle for given flow.
+ *
+ * At the moment we eat a raw IP route. Mostly to speed up lookups
+ * on interfaces with disabled IPsec.
+ */
+struct dst_entry *xfrm_lookup(struct net *net, struct dst_entry *dst_orig,
+ const struct flowi *fl, const struct sock *sk,
+ int flags)
+{
+ return xfrm_lookup_with_ifid(net, dst_orig, fl, sk, flags, 0);
+}
EXPORT_SYMBOL(xfrm_lookup);
/* Callers of xfrm_lookup_route() must ensure a call to dst_output().
@@ -2257,19 +2285,12 @@ int __xfrm_decode_session(struct sk_buff *skb, struct flowi *fl,
unsigned int family, int reverse)
{
const struct xfrm_policy_afinfo *afinfo = xfrm_policy_get_afinfo(family);
- const struct xfrm_if_cb *ifcb = xfrm_if_get_cb();
- struct xfrm_if *xi;
int err;
if (unlikely(afinfo == NULL))
return -EAFNOSUPPORT;
afinfo->decode_session(skb, fl, reverse);
- if (ifcb) {
- xi = ifcb->decode_session(skb);
- if (xi)
- fl->flowi_xfrm.if_id = xi->p.if_id;
- }
err = security_xfrm_decode_session(skb, &fl->flowi_secid);
rcu_read_unlock();
@@ -2301,6 +2322,19 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
int reverse;
struct flowi fl;
int xerr_idx = -1;
+ const struct xfrm_if_cb *ifcb;
+ struct xfrm_if *xi;
+ u32 if_id = 0;
+
+ rcu_read_lock();
+ ifcb = xfrm_if_get_cb();
+
+ if (ifcb) {
+ xi = ifcb->decode_session(skb);
+ if (xi)
+ if_id = xi->p.if_id;
+ }
+ rcu_read_unlock();
reverse = dir & ~XFRM_POLICY_MASK;
dir &= XFRM_POLICY_MASK;
@@ -2328,7 +2362,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
pol = NULL;
sk = sk_to_full_sk(sk);
if (sk && sk->sk_policy[dir]) {
- pol = xfrm_sk_policy_lookup(sk, dir, &fl, family);
+ pol = xfrm_sk_policy_lookup(sk, dir, &fl, family, if_id);
if (IS_ERR(pol)) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
return 0;
@@ -2336,7 +2370,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
}
if (!pol)
- pol = xfrm_policy_lookup(net, &fl, family, dir);
+ pol = xfrm_policy_lookup(net, &fl, family, dir, if_id);
if (IS_ERR(pol)) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
@@ -2360,7 +2394,7 @@ int __xfrm_policy_check(struct sock *sk, int dir, struct sk_buff *skb,
if (pols[0]->type != XFRM_POLICY_TYPE_MAIN) {
pols[1] = xfrm_policy_lookup_bytype(net, XFRM_POLICY_TYPE_MAIN,
&fl, family,
- XFRM_POLICY_IN);
+ XFRM_POLICY_IN, if_id);
if (pols[1]) {
if (IS_ERR(pols[1])) {
XFRM_INC_STATS(net, LINUX_MIB_XFRMINPOLERROR);
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 27c84e63c7ff..bd5cb7ad2447 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -930,7 +930,7 @@ struct xfrm_state *
xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
const struct flowi *fl, struct xfrm_tmpl *tmpl,
struct xfrm_policy *pol, int *err,
- unsigned short family)
+ unsigned short family, u32 if_id)
{
static xfrm_address_t saddr_wildcard = { };
struct net *net = xp_net(pol);
@@ -940,7 +940,6 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
int error = 0;
struct xfrm_state *best = NULL;
u32 mark = pol->mark.v & pol->mark.m;
- u32 if_id = fl->flowi_xfrm.if_id;
unsigned short encap_family = tmpl->encap_family;
unsigned int sequence;
struct km_event c;
--
2.18.0.233.g985f88cf7e-goog
^ permalink raw reply related
* Re: [PATCH net] net: rollback orig value on failure of dev_qdisc_change_tx_queue_len
From: Cong Wang @ 2018-07-19 17:25 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Miller, Linux Kernel Network Developers, eranbe
In-Reply-To: <1532010858-6183-1-git-send-email-tariqt@mellanox.com>
On Thu, Jul 19, 2018 at 7:34 AM Tariq Toukan <tariqt@mellanox.com> wrote:
>
> Fix dev_change_tx_queue_len so it rolls back original value
> upon a failure in dev_qdisc_change_tx_queue_len.
> This is already done for notifirers' failures, share the code.
>
> The revert of changes in dev_qdisc_change_tx_queue_len
> in such case is needed but missing (marked as TODO), yet it is
> still better to not apply the new requested value.
You misunderstand the TODO, that is for reverting tx queue len
change for previous queues in the loop. I still don't have any
nice solution for this yet.
Yeah, your change itself looks good.
Please update the changelog.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: remove redundant input checks in SIOCSIFTXQLEN case of dev_ifsioc
From: Cong Wang @ 2018-07-19 17:21 UTC (permalink / raw)
To: Tariq Toukan; +Cc: David Miller, Linux Kernel Network Developers, eranbe
In-Reply-To: <1532011832-6952-1-git-send-email-tariqt@mellanox.com>
On Thu, Jul 19, 2018 at 7:50 AM Tariq Toukan <tariqt@mellanox.com> wrote:
> --- a/net/core/dev_ioctl.c
> +++ b/net/core/dev_ioctl.c
> @@ -282,14 +282,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
> return dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
>
> case SIOCSIFTXQLEN:
> - if (ifr->ifr_qlen < 0)
> - return -EINVAL;
Are you sure we can remove this if check too?
The other one is safe to remove.
^ permalink raw reply
* Re: [PATCH] net: phy: sfp: Do not use "imply HWMON"
From: Andrew Lunn @ 2018-07-19 17:19 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Florian Fainelli, David S . Miller, netdev, linux-kernel
In-Reply-To: <1532018499-16490-1-git-send-email-linux@roeck-us.net>
On Thu, Jul 19, 2018 at 09:41:39AM -0700, Guenter Roeck wrote:
> "imply HWMON" was supposed to ensure that the SFP phy code can be built
> with HWMON enabled or disabled while at the same time ensuring that
> HWMON is not built as module if SFP is built into the kernel.
> Unfortunately, that does not work as intended. With "allmodconfig", it
> results in several unrelated HWMON drivers to be disabled instead of
> being built as module as expected.
>
> Let's use the old "depends on HWMON || HWMON=n" instead. This is slightly
> different (it enforces SFP to be built as module if HWMON is built as
> module), but it is better than the alternative of using "IS_REACHABLE()"
> in the driver since that would disable sensor support if HWMON is built
> as module and SFP is built into the kernel.
>
> Fixes: 1323061a018a ("net: phy: sfp: Add HWMON support for module sensors")
> Cc: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next 4/4] act_mirred: use ACT_REDIRECT when possible
From: kbuild test robot @ 2018-07-19 17:16 UTC (permalink / raw)
To: Paolo Abeni
Cc: kbuild-all, netdev, Jamal Hadi Salim, Cong Wang, Jiri Pirko,
Alexei Starovoitov, Daniel Borkmann, Marcelo Ricardo Leitner
In-Reply-To: <95034fe41fa132b2216686eeb41deaefd997e85b.1531473946.git.pabeni@redhat.com>
Hi Paolo,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Paolo-Abeni/TC-refactor-TC_ACT_REDIRECT-action/20180716-011055
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> net/sched/act_mirred.c:195:17: sparse: dubious: x | !y
net/sched/act_mirred.c:260:23: sparse: expression using sizeof(void)
net/sched/act_mirred.c:260:23: sparse: expression using sizeof(void)
vim +195 net/sched/act_mirred.c
169
170 static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a,
171 struct tcf_result *res)
172 {
173 struct tcf_mirred *m = to_mirred(a);
174 bool m_mac_header_xmit;
175 struct net_device *dev;
176 struct sk_buff *skb2;
177 int retval, err = 0;
178 bool want_ingress;
179 int m_eaction;
180 int mac_len;
181
182 tcf_lastuse_update(&m->tcf_tm);
183 bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);
184
185 m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);
186 m_eaction = READ_ONCE(m->tcfm_eaction);
187 retval = READ_ONCE(m->tcf_action);
188 dev = rcu_dereference_bh(m->tcfm_dev);
189 want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
190 if (skb_at_tc_ingress(skb) && tcf_mirred_is_act_redirect(m_eaction)) {
191 skb->tc_redirected = 1;
192 skb->tc_from_ingress = 1;
193
194 /* the core redirect code will check dev and its status */
> 195 TCF_RESULT_SET_REDIRECT(res, dev, want_ingress);
196 res->qstats = this_cpu_ptr(m->common.cpu_qstats);
197 return TC_ACT_REDIRECT;
198 }
199
200 if (unlikely(!dev)) {
201 pr_notice_once("tc mirred: target device is gone\n");
202 goto out;
203 }
204
205 if (unlikely(!(dev->flags & IFF_UP))) {
206 net_notice_ratelimited("tc mirred to Houston: device %s is down\n",
207 dev->name);
208 goto out;
209 }
210
211 skb2 = skb_clone(skb, GFP_ATOMIC);
212 if (!skb2)
213 goto out;
214
215 /* If action's target direction differs than filter's direction,
216 * and devices expect a mac header on xmit, then mac push/pull is
217 * needed.
218 */
219 if (skb_at_tc_ingress(skb) != want_ingress && m_mac_header_xmit) {
220 if (!skb_at_tc_ingress(skb)) {
221 /* caught at egress, act ingress: pull mac */
222 mac_len = skb_network_header(skb) - skb_mac_header(skb);
223 skb_pull_rcsum(skb2, mac_len);
224 } else {
225 /* caught at ingress, act egress: push mac */
226 skb_push_rcsum(skb2, skb->mac_len);
227 }
228 }
229
230 /* mirror is always swallowed */
231 if (tcf_mirred_is_act_redirect(m_eaction)) {
232 skb2->tc_redirected = 1;
233 skb2->tc_from_ingress = skb2->tc_at_ingress;
234 }
235
236 skb2->skb_iif = skb->dev->ifindex;
237 skb2->dev = dev;
238 if (!tcf_mirred_act_wants_ingress(m_eaction))
239 err = dev_queue_xmit(skb2);
240 else
241 err = netif_receive_skb(skb2);
242
243 if (err) {
244 out:
245 qstats_overlimit_inc(this_cpu_ptr(m->common.cpu_qstats));
246 if (tcf_mirred_is_act_redirect(m_eaction))
247 retval = TC_ACT_SHOT;
248 }
249
250 return retval;
251 }
252
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH iproute2/next 1/2] tc/act_tunnel_key: Enable setup of tos and ttl
From: Or Gerlitz @ 2018-07-19 17:14 UTC (permalink / raw)
To: Roman Mashak; +Cc: Or Gerlitz, David Ahern, Linux Netdev List
In-Reply-To: <85fu0fbg4j.fsf@mojatatu.com>
On Thu, Jul 19, 2018 at 2:48 PM, Roman Mashak <mrv@mojatatu.com> wrote:
> Or Gerlitz <ogerlitz@mellanox.com> writes:
>
>> Allow to set tos and ttl for the tunnel.
>>
>> For example, here's encap rule that sets tos to the tunnel:
>>
>> tc filter add dev eth0_0 protocol ip parent ffff: prio 10 flower \
>> src_mac e4:11:22:33:44:50 dst_mac e4:11:22:33:44:70 \
>> action tunnel_key set src_ip 192.168.10.1 dst_ip 192.168.10.2 id 100 dst_port 4789 tos 0x30 \
>> action mirred egress redirect dev vxlan_sys_4789
>>
>> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
>> Reviewed-by: Roi Dayan <roid@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>
> [...]
>
> Or, could you also update tunnel_key actions for the new options in
> $(kernel)/tools/testing/selftests/tc-testing/tc-tests/actions/tunnel_key.json
> once the patches are accepted ?
yes, I will do that
^ permalink raw reply
* Re: [PATCH 4/5] net: add support for nvmem to eth_platform_get_mac_address()
From: Russell King - ARM Linux @ 2018-07-19 17:47 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Sekhar Nori, Kevin Hilman, Grygorii Strashko, David S . Miller,
Srinivas Kandagatla, Lukas Wunner, Rob Herring, Florian Fainelli,
Dan Carpenter, Ivan Khoronzhuk, David Lechner, Greg Kroah-Hartman,
Andrew Lunn, linux-arm-kernel, linux-kernel, linux-omap, netdev,
Bartosz Golaszewski
In-Reply-To: <20180718161035.7005-5-brgl@bgdev.pl>
On Wed, Jul 18, 2018 at 06:10:34PM +0200, Bartosz Golaszewski wrote:
> @@ -544,6 +548,31 @@ int eth_platform_get_mac_address(struct device *dev, u8 *mac_addr)
> from = "arch callback";
> }
>
> + if (!addr) {
> + nvmem = nvmem_cell_get(dev, "mac-address");
> + if (IS_ERR(nvmem) && PTR_ERR(nvmem) == -EPROBE_DEFER)
This is way too verbose. To quote Al Viro from earlier today:
<viro> sigh...
<viro> if (IS_ERR(link) && PTR_ERR(link) == -EEXIST)
<viro> what the hell is wrong with if (link == ERR_PTR(-EEXIST))?
I wonder why so many people haven't heard of pointer comparison... ;)
IS_ERR(ERR_PTR(-EPROBE_DEFER)) is always true - if it wasn't, then
we'd be in for problems.
So, if you're asserting that nvmem is ERR_PTR(-EPROBE_DEFER) then
there's no need to do the IS_ERR(nvmem) must also be true. Hence, a
simple pointer comparison is sufficient:
if (nvmem == ERR_PTR(-EPROBE_DEFER))
return -EPROBE_DEFER;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* [PATCH iproute2-next] q_cake: Rename autorate_ingress parameter to use dash as word separator
From: Toke Høiland-Jørgensen @ 2018-07-19 16:55 UTC (permalink / raw)
To: dsahern; +Cc: netdev, cake, Toke Høiland-Jørgensen
This is consistent with the other multi-word parameters. Also change the
JSON output to be consistent with way it is formatted for the other
options.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
man/man8/tc-cake.8 | 4 ++--
tc/q_cake.c | 8 ++++----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/man/man8/tc-cake.8 b/man/man8/tc-cake.8
index 0ffef094..0e84bc6e 100644
--- a/man/man8/tc-cake.8
+++ b/man/man8/tc-cake.8
@@ -9,7 +9,7 @@ CAKE \- Common Applications Kept Enhanced (CAKE)
RATE |
.BR unlimited*
|
-.BR autorate_ingress
+.BR autorate-ingress
]
.br
[
@@ -149,7 +149,7 @@ RATE
.BR tc(8)
or examples below for details of the RATE value.
.PP
-.B autorate_ingress
+.B autorate-ingress
.br
Automatic capacity estimation based on traffic arriving at this qdisc.
This is most likely to be useful with cellular links, which tend to change
diff --git a/tc/q_cake.c b/tc/q_cake.c
index b5476584..f1e232a6 100644
--- a/tc/q_cake.c
+++ b/tc/q_cake.c
@@ -71,7 +71,7 @@ static struct cake_preset *find_preset(char *argv)
static void explain(void)
{
fprintf(stderr,
-"Usage: ... cake [ bandwidth RATE | unlimited* | autorate_ingress ]\n"
+"Usage: ... cake [ bandwidth RATE | unlimited* | autorate-ingress ]\n"
" [ rtt TIME | datacentre | lan | metro | regional |\n"
" internet* | oceanic | satellite | interplanetary ]\n"
" [ besteffort | diffserv8 | diffserv4 | diffserv3* ]\n"
@@ -122,7 +122,7 @@ static int cake_parse_opt(struct qdisc_util *qu, int argc, char **argv,
bandwidth = 0;
unlimited = 1;
autorate = 0;
- } else if (strcmp(*argv, "autorate_ingress") == 0) {
+ } else if (strcmp(*argv, "autorate-ingress") == 0) {
autorate = 1;
} else if (strcmp(*argv, "rtt") == 0) {
NEXT_ARG();
@@ -435,8 +435,8 @@ static int cake_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
RTA_PAYLOAD(tb[TCA_CAKE_AUTORATE]) >= sizeof(__u32)) {
autorate = rta_getattr_u32(tb[TCA_CAKE_AUTORATE]);
if (autorate == 1)
- print_string(PRINT_ANY, "autorate", "autorate_%s ",
- "ingress");
+ print_string(PRINT_ANY, "autorate", "%s ",
+ "autorate-ingress");
else if (autorate)
print_string(PRINT_ANY, "autorate", "(?autorate?) ",
"unknown");
--
2.18.0
^ permalink raw reply related
* Re: [PATCH RFC bpf-next] bpf: per-register parent pointers
From: Edward Cree @ 2018-07-19 16:54 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: davem, daniel, netdev
In-Reply-To: <20180718035447.1443811-1-ast@kernel.org>
On 18/07/18 04:54, Alexei Starovoitov wrote:
> I'd like to apply it, but I see the difference in insn_processed.
> Several cilium tests show favorable difference towards new liveness approach.
> selftests/bpf/test_xdp_noinline.o also shows the difference.
> I'm struggling to see why this patch would make such difference.
> Could you please help me analyze why such difference exists?
I'm also confused by it at present, but I am looking into it.
-Ed
^ permalink raw reply
* Re: [PATCH iproute2-next v11] Add support for CAKE qdisc
From: Toke Høiland-Jørgensen @ 2018-07-19 16:47 UTC (permalink / raw)
To: David Ahern, netdev; +Cc: cake, Dave Taht
In-Reply-To: <b1dc9305-ff72-d84a-aacf-3a7a69ea6b7c@gmail.com>
David Ahern <dsahern@gmail.com> writes:
> On 7/19/18 7:56 AM, Toke Høiland-Jørgensen wrote:
>> sch_cake is intended to squeeze the most bandwidth and latency out of even
>> the slowest ISP links and routers, while presenting an API simple enough
>> that even an ISP can configure it.
>>
>> Example of use on a cable ISP uplink:
>>
>> tc qdisc add dev eth0 cake bandwidth 20Mbit nat docsis ack-filter
>>
>> To shape a cable download link (ifb and tc-mirred setup elided)
>>
>> tc qdisc add dev ifb0 cake bandwidth 200mbit nat docsis ingress wash besteffort
>>
>> Cake is filled with:
>>
>> * A hybrid Codel/Blue AQM algorithm, "Cobalt", tied to an FQ_Codel
>> derived Flow Queuing system, which autoconfigures based on the bandwidth.
>> * A novel "triple-isolate" mode (the default) which balances per-host
>> and per-flow FQ even through NAT.
>> * An deficit based shaper, that can also be used in an unlimited mode.
>> * 8 way set associative hashing to reduce flow collisions to a minimum.
>> * A reasonable interpretation of various diffserv latency/loss tradeoffs.
>> * Support for zeroing diffserv markings for entering and exiting traffic.
>> * Support for interacting well with Docsis 3.0 shaper framing.
>> * Support for DSL framing types and shapers.
>> * Support for ack filtering.
>> * Extensive statistics for measuring, loss, ecn markings, latency variation.
>>
>> Various versions baking have been available as an out of tree build for
>> kernel versions going back to 3.10, as the embedded router world has been
>> running a few years behind mainline Linux. A stable version has been
>> generally available on lede-17.01 and later.
>>
>> sch_cake replaces a combination of iptables, tc filter, htb and fq_codel
>> in the sqm-scripts, with sane defaults and vastly simpler configuration.
>>
>> Cake's principal author is Jonathan Morton, with contributions from
>> Kevin Darbyshire-Bryant, Toke Høiland-Jørgensen, Sebastian Moeller,
>> Ryan Mounce, Tony Ambardar, Dean Scarff, Nils Andreas Svee, Dave Täht,
>> and Loganaden Velvindron.
>>
>> Testing from Pete Heist, Georgios Amanakis, and the many other members of
>> the cake@lists.bufferbloat.net mailing list.
>>
>> Signed-off-by: Dave Taht <dave.taht@gmail.com>
>> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
>> ---
>
> applied to iproute2-next. Thanks
Great, thanks!
> About the autorate, I think you should consider consistency in output.
Yeah, I'd tend to agree. Did actually send an updated patch, but guess
the streams crossed. Can send a separate patch with the rename; I see I
forgot to update the man page anyway...
-Toke
^ permalink raw reply
* Re: [PATCH iproute2 5/5] bpf: implement btf handling and map annotation
From: Martin KaFai Lau @ 2018-07-19 16:37 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jakub Kicinski, dsahern, alexei.starovoitov, netdev
In-Reply-To: <d52ff3cf-b12e-e3bf-cf04-956bdf8c3272@iogearbox.net>
On Thu, Jul 19, 2018 at 05:43:11PM +0200, Daniel Borkmann wrote:
> On 07/19/2018 02:11 AM, Martin KaFai Lau wrote:
> > On Wed, Jul 18, 2018 at 11:13:37AM -0700, Jakub Kicinski wrote:
> >> On Wed, 18 Jul 2018 11:33:22 +0200, Daniel Borkmann wrote:
> >>> On 07/18/2018 10:42 AM, Daniel Borkmann wrote:
> >>>> On 07/18/2018 02:27 AM, Jakub Kicinski wrote:
> >>>>> On Wed, 18 Jul 2018 01:31:22 +0200, Daniel Borkmann wrote:
> >>>>>> # bpftool map dump id 386
> >>>>>> [{
> >>>>>> "key": 0,
> >>>>>> "value": {
> >>>>>> "": {
> >>>>>> "value": 0,
> >>>>>> "ifindex": 0,
> >>>>>> "mac": []
> >>>>>> }
> >>>>>> }
> >>>>>> },{
> >>>>>> "key": 1,
> >>>>>> "value": {
> >>>>>> "": {
> >>>>>> "value": 0,
> >>>>>> "ifindex": 0,
> >>>>>> "mac": []
> >>>>>> }
> >>>>>> }
> >>>>>> },{
> >>>>>> [...]
> >>>>>
> >>>>> Ugh, the empty keys ("") look worrying, we should probably improve
> >>>>> handling of anonymous structs in bpftool :S
> >>>>
> >>>> Yeah agree, I think it would be nice to see a more pahole style dump
> >>>> where we have types and member names along with the value as otherwise
> >>>> it might be a bit confusing.
> >>>
> >>> Another feature that would be super useful imho would be in the /single/
> >>> map view e.g. 'bpftool map show id 123' to have a detailed BTF key+value
> >>> type dump, so in addition to the basic map info we show pahole like info
> >>> of the structs with length/offsets.
> >>
> >> That sounds good! We could also consider adding a btf object and
> >> commands to interrogate BTF types in the kernel in general.. Perhaps
> >> then we could add something like bpftool btf describe map id 123.
> > +1 on the btf subcommand.
>
> That would also work, I think both might be useful to have. Former would
> all sit under a single command to show map details.
Agree that both would be useful. btf command could address the whole BTF
object which could include many maps/types while the map command is
focusing on its own map info.
> With 'bpftool btf' you
> would also allow for a full BTF dump when a specific BTF obj id is provided?
Right, I think the BTF obj id (or file) is needed for the btf command. and then
it should allow to do full dump or only show a particular map/type id.
A little forward thinking, map here is a C type. Hence, I think using the
name "type" like "btftool btf id 1 show _type_ id 123" may be better when
we later expand BTF usage beyond BPF program.
>
> >> Having the single map view show more information seems interesting, but
> >> I wonder if it could be surprising. Is there precedent for such
> >> behaviour?
> > Having everything in one page (map show id 123) could be interesting.
> > One thing is the pahole-like output may be quite long?
> > e.g. the member of a struct could itself be another struct.
>
> Right, though probably fine when you want to see all information specific
> to one map. Of course the 'bpftool map' list view would need to hide this
> information.
>
> > Not sure how the pahole-like output may look like in json though.
>
> Would the existing map k/v dump have more or less the same 'issue'?
True, the existing map k/v dump of the map data is reusing the
json {}/[]/""/number convention. I think that is ok and actually a
pretty condensed way since people are used to this convention when
reading "data" output.
For printing out C type, I think it is more natural to have it as
close to C syntax as possible in order to have it parsable by human
eyes. However, yes, we could reuse a similar fashion to print type
in json as we do in printing data. Just curious, the json type output
is more for script or mostly for people that can read everything from one
json output.
For plaintext, we can just print like pahole.
^ permalink raw reply
* Re: [PATCH RFC/RFT net-next 00/17] net: Convert neighbor tables to per-namespace
From: Cong Wang @ 2018-07-19 17:12 UTC (permalink / raw)
To: David Ahern
Cc: David Miller, Linux Kernel Network Developers, nikita.leshchenko,
Roopa Prabhu, Stephen Hemminger, Ido Schimmel, Jiri Pirko,
Saeed Mahameed, Alexander Aring, linux-wpan, NetFilter, LKML
In-Reply-To: <28c30574-391c-b4bd-c337-51d3040d901a@gmail.com>
On Thu, Jul 19, 2018 at 9:16 AM David Ahern <dsahern@gmail.com> wrote:
>
> Chatting with Nikolay about this and he brought up a good corollary - ip
> fragmentation. It really is a similar problem in that memory is consumed
> as a result of packets received from an external entity. The ipfrag
> sysctls are per namespace with a limit that non-init_net namespaces can
> not set high_thresh > the current value of init_net. Potential memory
> consumed by fragments scales with the number of namespaces which is the
> primary concern with making neighbor tables per namespace.
Nothing new, already discussed:
https://marc.info/?l=linux-netdev&m=140391416215988&w=2
:)
^ permalink raw reply
* Re: [PATCH iproute2-next v11] Add support for CAKE qdisc
From: David Ahern @ 2018-07-19 16:25 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, netdev; +Cc: cake, Dave Taht
In-Reply-To: <20180719135617.31850-1-toke@toke.dk>
On 7/19/18 7:56 AM, Toke Høiland-Jørgensen wrote:
> sch_cake is intended to squeeze the most bandwidth and latency out of even
> the slowest ISP links and routers, while presenting an API simple enough
> that even an ISP can configure it.
>
> Example of use on a cable ISP uplink:
>
> tc qdisc add dev eth0 cake bandwidth 20Mbit nat docsis ack-filter
>
> To shape a cable download link (ifb and tc-mirred setup elided)
>
> tc qdisc add dev ifb0 cake bandwidth 200mbit nat docsis ingress wash besteffort
>
> Cake is filled with:
>
> * A hybrid Codel/Blue AQM algorithm, "Cobalt", tied to an FQ_Codel
> derived Flow Queuing system, which autoconfigures based on the bandwidth.
> * A novel "triple-isolate" mode (the default) which balances per-host
> and per-flow FQ even through NAT.
> * An deficit based shaper, that can also be used in an unlimited mode.
> * 8 way set associative hashing to reduce flow collisions to a minimum.
> * A reasonable interpretation of various diffserv latency/loss tradeoffs.
> * Support for zeroing diffserv markings for entering and exiting traffic.
> * Support for interacting well with Docsis 3.0 shaper framing.
> * Support for DSL framing types and shapers.
> * Support for ack filtering.
> * Extensive statistics for measuring, loss, ecn markings, latency variation.
>
> Various versions baking have been available as an out of tree build for
> kernel versions going back to 3.10, as the embedded router world has been
> running a few years behind mainline Linux. A stable version has been
> generally available on lede-17.01 and later.
>
> sch_cake replaces a combination of iptables, tc filter, htb and fq_codel
> in the sqm-scripts, with sane defaults and vastly simpler configuration.
>
> Cake's principal author is Jonathan Morton, with contributions from
> Kevin Darbyshire-Bryant, Toke Høiland-Jørgensen, Sebastian Moeller,
> Ryan Mounce, Tony Ambardar, Dean Scarff, Nils Andreas Svee, Dave Täht,
> and Loganaden Velvindron.
>
> Testing from Pete Heist, Georgios Amanakis, and the many other members of
> the cake@lists.bufferbloat.net mailing list.
>
> Signed-off-by: Dave Taht <dave.taht@gmail.com>
> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
> ---
applied to iproute2-next. Thanks
About the autorate, I think you should consider consistency in output.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox