* Re: [PATCH net-next 01/12] bpf: verifier: set reg_type on context accesses in second pass
From: Jakub Kicinski @ 2017-10-12 20:56 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, oss-drivers, alexei.starovoitov
In-Reply-To: <59DFD3DE.9050907@iogearbox.net>
On Thu, 12 Oct 2017 22:43:10 +0200, Daniel Borkmann wrote:
> On 10/12/2017 07:34 PM, Jakub Kicinski wrote:
> > Use a simplified is_valid_access() callback when verifier
> > is used for program analysis by non-host JITs. This allows
> > us to teach the verifier about packet start and packet end
> > offsets for direct packet access.
> >
> > We can extend the callback as needed but for most packet
> > processing needs there isn't much more the offloads may
> > require.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > ---
> > CC: alexei.starovoitov@gmail.com
> > CC: daniel@iogearbox.net
> >
> > kernel/bpf/verifier.c | 43 +++++++++++++++++++++++++++++++++++++------
> > 1 file changed, 37 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 2cdbcc4f8f6b..9755279d94cb 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -813,6 +813,36 @@ static int check_packet_access(struct bpf_verifier_env *env, u32 regno, int off,
> > return err;
> > }
> >
> > +static bool analyzer_is_valid_access(struct bpf_verifier_env *env, int off,
> > + struct bpf_insn_access_aux *info)
> > +{
> > + switch (env->prog->type) {
> > + case BPF_PROG_TYPE_XDP:
> > + switch (off) {
> > + case offsetof(struct xdp_buff, data):
> > + info->reg_type = PTR_TO_PACKET;
> > + return true;
> > + case offsetof(struct xdp_buff, data_end):
> > + info->reg_type = PTR_TO_PACKET_END;
> > + return true;
> > + }
> > + return false;
> > + case BPF_PROG_TYPE_SCHED_CLS:
> > + switch (off) {
> > + case offsetof(struct sk_buff, data):
> > + info->reg_type = PTR_TO_PACKET;
> > + return true;
> > + case offsetof(struct sk_buff, cb) +
> > + offsetof(struct bpf_skb_data_end, data_end):
> > + info->reg_type = PTR_TO_PACKET_END;
> > + return true;
> > + }
> > + return false;
> > + default:
> > + return false;
> > + }
> > +}
> > +
> > /* check access to 'struct bpf_context' fields. Supports fixed offsets only */
> > static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off, int size,
> > enum bpf_access_type t, enum bpf_reg_type *reg_type)
> > @@ -821,12 +851,13 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, int off,
> > .reg_type = *reg_type,
> > };
> >
> > - /* for analyzer ctx accesses are already validated and converted */
> > - if (env->analyzer_ops)
> > - return 0;
> > -
> > - if (env->prog->aux->ops->is_valid_access &&
> > - env->prog->aux->ops->is_valid_access(off, size, t, &info)) {
> > + if (env->analyzer_ops) {
> > + if (analyzer_is_valid_access(env, off, &info)) {
> > + *reg_type = info.reg_type;
>
> Is there some specific issue with the is_valid_access() callbacks that you
> need to do this (I couldn't parse that out of the commit message)?
Do you mean why not just call is_valid_access()? The offsets are
translated, so is_valid_access() will use user space __sk_buff's
offsets while we have the kernel's sk_buff offsets here...
> It would be nice to keep the reg_type setting in one place, meaning
> the callbacks themselves, so we wouldn't need to maintain this in
> multiple places.
Hm.. I though this was the smallest and simplest change. I could
translate the offsets but that seems wobbly. Or try to consolidate the
call into the same if () branch? Not sure..
As a bonus info I discovered there is a bug in -net with how things are
converted. We allow arithmetic on context pointers but then only
look at the insn.off in the converter... I'm working on a fix.
^ permalink raw reply
* Re: [net-next V7 PATCH 3/5] bpf: cpumap xdp_buff to skb conversion and allocation
From: Edward Cree @ 2017-10-12 21:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer, netdev
Cc: jakub.kicinski, Michael S. Tsirkin, pavel.odintsov, Jason Wang,
mchan, John Fastabend, peter.waskiewicz.jr, ast, Daniel Borkmann,
Alexei Starovoitov, Andy Gospodarek
In-Reply-To: <150781121989.9409.14542782810603037426.stgit@firesoul>
On 12/10/17 13:26, Jesper Dangaard Brouer wrote:
> This patch makes cpumap functional, by adding SKB allocation and
> invoking the network stack on the dequeuing CPU.
>
> For constructing the SKB on the remote CPU, the xdp_buff in converted
> into a struct xdp_pkt, and it mapped into the top headroom of the
> packet, to avoid allocating separate mem. For now, struct xdp_pkt is
> just a cpumap internal data structure, with info carried between
> enqueue to dequeue.
<snip>
> +struct sk_buff *cpu_map_build_skb(struct bpf_cpu_map_entry *rcpu,
> + struct xdp_pkt *xdp_pkt)
> +{
> + unsigned int frame_size;
> + void *pkt_data_start;
> + struct sk_buff *skb;
> +
> + /* build_skb need to place skb_shared_info after SKB end, and
> + * also want to know the memory "truesize". Thus, need to
> + * know the memory frame size backing xdp_buff.
> + *
> + * XDP was designed to have PAGE_SIZE frames, but this
> + * assumption is not longer true with ixgbe and i40e. It
> + * would be preferred to set frame_size to 2048 or 4096
> + * depending on the driver.
> + * frame_size = 2048;
> + * frame_len = frame_size - sizeof(*xdp_pkt);
> + *
> + * Instead, with info avail, skb_shared_info in placed after
> + * packet len. This, unfortunately fakes the truesize.
> + * Another disadvantage of this approach, the skb_shared_info
> + * is not at a fixed memory location, with mixed length
> + * packets, which is bad for cache-line hotness.
> + */
> + frame_size = SKB_DATA_ALIGN(xdp_pkt->len) + xdp_pkt->headroom +
> + SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
> +
> + pkt_data_start = xdp_pkt->data - xdp_pkt->headroom;
> + skb = build_skb(pkt_data_start, frame_size);
> + if (!skb)
> + return NULL;
> +
> + skb_reserve(skb, xdp_pkt->headroom);
> + __skb_put(skb, xdp_pkt->len);
> + if (xdp_pkt->metasize)
> + skb_metadata_set(skb, xdp_pkt->metasize);
> +
> + /* Essential SKB info: protocol and skb->dev */
> + skb->protocol = eth_type_trans(skb, xdp_pkt->dev_rx);
> +
> + /* Optional SKB info, currently missing:
> + * - HW checksum info (skb->ip_summed)
> + * - HW RX hash (skb_set_hash)
> + * - RX ring dev queue index (skb_record_rx_queue)
> + */
One possibility for dealing with these and related issues — also things
like the proper way to free an xdp_buff if SKB creation fails, which
might not be page_frag_free() for some drivers with unusual recycle ring
implementations — is to have a new ndo for 'receiving' an xdp_pkt from a
cpumap redirect.
Since you're always receiving from the same driver that enqueued it, even
the structure of the metadata stored in the top of the packet page
doesn't have to be standardised; instead, each driver can put there just
whatever happens to be needed for its ndo_xdp_rx routine. (Though there
would probably be standard enqueue and dequeue functions that the
'common-case' drivers could use.)
In some cases, the driver could even just leave in the page the packet
prefix it got from the NIC, rather than reading it and then writing an
interpreted version back, thus minimising the number of packet-page
cachelines the 'bottom half' RX function has to touch (it would still
need to write in anything it got from the RX event, of course).
It shouldn't be much work as many driver RX routines are already
structured this way — sfc, for instance, has a split into efx_rx_packet()
and __efx_rx_packet(), as a software pipeline for prefetching.
-Ed
^ permalink raw reply
* [PATCH] atm: fore200e: mark expected switch fall-throughs
From: Gustavo A. R. Silva @ 2017-10-12 21:11 UTC (permalink / raw)
To: Chas Williams
Cc: linux-atm-general, netdev, linux-kernel, Gustavo A. R. Silva
In preparation to enabling -Wimplicit-fallthrough, mark switch cases
where we are expecting to fall through.
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
drivers/atm/fore200e.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index f8b7e86..126855e 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -358,26 +358,33 @@ fore200e_shutdown(struct fore200e* fore200e)
case FORE200E_STATE_COMPLETE:
kfree(fore200e->stats);
+ /* fall through */
case FORE200E_STATE_IRQ:
free_irq(fore200e->irq, fore200e->atm_dev);
+ /* fall through */
case FORE200E_STATE_ALLOC_BUF:
fore200e_free_rx_buf(fore200e);
+ /* fall through */
case FORE200E_STATE_INIT_BSQ:
fore200e_uninit_bs_queue(fore200e);
+ /* fall through */
case FORE200E_STATE_INIT_RXQ:
fore200e->bus->dma_chunk_free(fore200e, &fore200e->host_rxq.status);
fore200e->bus->dma_chunk_free(fore200e, &fore200e->host_rxq.rpd);
+ /* fall through */
case FORE200E_STATE_INIT_TXQ:
fore200e->bus->dma_chunk_free(fore200e, &fore200e->host_txq.status);
fore200e->bus->dma_chunk_free(fore200e, &fore200e->host_txq.tpd);
+ /* fall through */
case FORE200E_STATE_INIT_CMDQ:
fore200e->bus->dma_chunk_free(fore200e, &fore200e->host_cmdq.status);
+ /* fall through */
case FORE200E_STATE_INITIALIZE:
/* nothing to do for that state */
@@ -390,6 +397,7 @@ fore200e_shutdown(struct fore200e* fore200e)
case FORE200E_STATE_MAP:
fore200e->bus->unmap(fore200e);
+ /* fall through */
case FORE200E_STATE_CONFIGURE:
/* nothing to do for that state */
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 01/12] bpf: verifier: set reg_type on context accesses in second pass
From: Daniel Borkmann @ 2017-10-12 21:33 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov
In-Reply-To: <20171012135632.1fa408bc@cakuba.netronome.com>
On 10/12/2017 10:56 PM, Jakub Kicinski wrote:
> On Thu, 12 Oct 2017 22:43:10 +0200, Daniel Borkmann wrote:
[...]
>> It would be nice to keep the reg_type setting in one place, meaning
>> the callbacks themselves, so we wouldn't need to maintain this in
>> multiple places.
>
> Hm.. I though this was the smallest and simplest change. I could
> translate the offsets but that seems wobbly. Or try to consolidate the
> call into the same if () branch? Not sure..
Different callbacks for post-verification would be good at min as it
would allow to keep all the context access info in one place for a
given type at least.
> As a bonus info I discovered there is a bug in -net with how things are
> converted. We allow arithmetic on context pointers but then only
> look at the insn.off in the converter... I'm working on a fix.
Ohh well, good catch, indeed! :( Can you also add coverage to the
bpf selftests for this?
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC 0/3] Adding config get/set to devlink
From: Michal Kubecek @ 2017-10-12 21:36 UTC (permalink / raw)
To: Florian Fainelli
Cc: David Miller, jiri, roopa, steven.lin1, netdev, jiri,
michael.chan, linux-pci, linville, gospo
In-Reply-To: <21ab4a5d-0b6a-7976-7bf0-acd334f2613f@gmail.com>
On Thu, Oct 12, 2017 at 12:20:07PM -0700, Florian Fainelli wrote:
> On 10/12/2017 12:06 PM, David Miller wrote:
> >
> > One suggestion is that devlink is used for getting ethtool stats for
> > objects lacking netdev representor's, and a new genetlink family is
> > used for netdev based ethtool.
>
> Right, I was also thinking along those lines that we we would have a new
> generic netlink family for ethtool to support ethtool over netlink.
This is what I plan to work on on next SUSE Hackweek in November. But
I'm, of course, open to suggestions and I don't insist on this approach.
Michal Kubecek
^ permalink raw reply
* Re: [patch net-next 00/34] net: sched: allow qdiscs to share filter block instances
From: David Ahern @ 2017-10-12 21:37 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, alexander.h.duyck,
john.fastabend, willemb
In-Reply-To: <20171012171823.1431-1-jiri@resnulli.us>
On 10/12/17 11:17 AM, Jiri Pirko wrote:
> So back to the example. First, we create 2 qdiscs. Both will share
> block number 22. "22" is just an identification. If we don't pass any
> block number, a new one will be generated by kernel:
>
> $ tc qdisc add dev ens7 ingress block 22
> ^^^^^^^^
> $ tc qdisc add dev ens8 ingress block 22
> ^^^^^^^^
>
> Now if we list the qdiscs, we will see the block index in the output:
>
> $ tc qdisc
> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>
> Now we can add filter to any of qdiscs sharing the same block:
>
> $ tc filter add dev ens7 parent ffff: protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>
>
> We will see the same output if we list filters for ens7 and ens8, including stats:
>
> $ tc -s filter show dev ens7 ingress
> filter protocol ip pref 25 flower chain 0
> filter protocol ip pref 25 flower chain 0 handle 0x1
> eth_type ipv4
> dst_ip 192.168.0.0/16
> not_in_hw
> action order 1: gact action drop
> random type none pass val 0
> index 1 ref 1 bind 1 installed 39 sec used 2 sec
> Action statistics:
> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> $ tc -s filter show dev ens8 ingress
> filter protocol ip pref 25 flower chain 0
> filter protocol ip pref 25 flower chain 0 handle 0x1
> eth_type ipv4
> dst_ip 192.168.0.0/16
> not_in_hw
> action order 1: gact action drop
> random type none pass val 0
> index 1 ref 1 bind 1 installed 40 sec used 3 sec
> Action statistics:
> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
This seems like really odd semantics to me ... a filter added to one
device shows up on another.
Why not make the shared block a standalone object that is configured
through its own set of commands and then referenced by both devices?
^ permalink raw reply
* Re: [PATCH net-next 01/12] bpf: verifier: set reg_type on context accesses in second pass
From: Jakub Kicinski @ 2017-10-12 21:39 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, oss-drivers, alexei.starovoitov
In-Reply-To: <59DFDFA1.3020809@iogearbox.net>
On Thu, 12 Oct 2017 23:33:21 +0200, Daniel Borkmann wrote:
> On 10/12/2017 10:56 PM, Jakub Kicinski wrote:
> > On Thu, 12 Oct 2017 22:43:10 +0200, Daniel Borkmann wrote:
> [...]
> >> It would be nice to keep the reg_type setting in one place, meaning
> >> the callbacks themselves, so we wouldn't need to maintain this in
> >> multiple places.
> >
> > Hm.. I though this was the smallest and simplest change. I could
> > translate the offsets but that seems wobbly. Or try to consolidate the
> > call into the same if () branch? Not sure..
>
> Different callbacks for post-verification would be good at min as it
> would allow to keep all the context access info in one place for a
> given type at least.
Sorry to be clear - you're suggesting adding a new callback to struct
bpf_verifier_ops, or swapping the struct bpf_verifier_ops for a
special post-verification one?
> > As a bonus info I discovered there is a bug in -net with how things are
> > converted. We allow arithmetic on context pointers but then only
> > look at the insn.off in the converter... I'm working on a fix.
>
> Ohh well, good catch, indeed! :( Can you also add coverage to the
> bpf selftests for this?
Will do!
^ permalink raw reply
* Re: Ethtool question
From: Ben Greear @ 2017-10-12 21:45 UTC (permalink / raw)
To: David Miller, linville; +Cc: netdev
In-Reply-To: <20171011.134919.321292333200236097.davem@davemloft.net>
On 10/11/2017 01:49 PM, David Miller wrote:
> From: "John W. Linville" <linville@tuxdriver.com>
> Date: Wed, 11 Oct 2017 16:44:07 -0400
>
>> On Wed, Oct 11, 2017 at 09:51:56AM -0700, Ben Greear wrote:
>>> I noticed today that setting some ethtool settings to the same value
>>> returns an error code. I would think this should silently return
>>> success instead? Makes it easier to call it from scripts this way:
>>>
>>> [root@lf0313-6477 lanforge]# ethtool -L eth3 combined 1
>>> combined unmodified, ignoring
>>> no channel parameters changed, aborting
>>> current values: tx 0 rx 0 other 1 combined 1
>>> [root@lf0313-6477 lanforge]# echo $?
>>> 1
>>
>> I just had this discussion a couple of months ago with someone. My
>> initial feeling was like you, a no-op is not a failure. But someone
>> convinced me otherwise...I will now endeavour to remember who that
>> was and how they convinced me...
>>
>> Anyone else have input here?
>
> I guess this usually happens when drivers don't support changing the
> settings at all. So they just make their ethtool operation for the
> 'set' always return an error.
>
> We could have a generic ethtool helper that does "get" and then if the
> "set" request is identical just return zero.
>
> But from another perspective, the error returned from the "set" in this
> situation also indicates to the user that the driver does not support
> the "set" operation which has value and meaning in and of itself. And
> we'd lose that with the given suggestion.
In my case, the driver (igb) does support the set, my program just made the same
ethtool call several times and it fails after the initial change (that actually
changes something), as best as I can figure.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [patch net-next 06/34] net: core: use dev->ingress_queue instead of tp->q
From: Daniel Borkmann @ 2017-10-12 21:45 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, simon.horman,
pieter.jansenvanvuuren, john.hurley, edumazet, dsahern,
alexander.h.duyck, john.fastabend, willemb
In-Reply-To: <20171012171823.1431-7-jiri@resnulli.us>
On 10/12/2017 07:17 PM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> In sch_handle_egress and sch_handle_ingress, don't use tp->q and use
> dev->ingress_queue which stores the same pointer instead.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> net/core/dev.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index fcddccb..cb9e5e5 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3273,14 +3273,18 @@ EXPORT_SYMBOL(dev_loopback_xmit);
> static struct sk_buff *
> sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> {
> + struct netdev_queue *netdev_queue =
> + rcu_dereference_bh(dev->ingress_queue);
> struct tcf_proto *cl = rcu_dereference_bh(dev->egress_cl_list);
> struct tcf_result cl_res;
> + struct Qdisc *q;
>
> - if (!cl)
> + if (!cl || !netdev_queue)
> return skb;
> + q = netdev_queue->qdisc;
NAK, no additional overhead in the software fast-path of
sch_handle_{ingress,egress}() like this. There are users out there
that use tc in software only, so performance is critical here.
> /* qdisc_skb_cb(skb)->pkt_len was already set by the caller. */
> - qdisc_bstats_cpu_update(cl->q, skb);
> + qdisc_bstats_cpu_update(q, skb);
>
> switch (tcf_classify(skb, cl, &cl_res, false)) {
> case TC_ACT_OK:
> @@ -3288,7 +3292,7 @@ sch_handle_egress(struct sk_buff *skb, int *ret, struct net_device *dev)
> skb->tc_index = TC_H_MIN(cl_res.classid);
> break;
> case TC_ACT_SHOT:
> - qdisc_qstats_cpu_drop(cl->q);
> + qdisc_qstats_cpu_drop(q);
> *ret = NET_XMIT_DROP;
> kfree_skb(skb);
> return NULL;
> @@ -4188,16 +4192,21 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
> struct net_device *orig_dev)
> {
> #ifdef CONFIG_NET_CLS_ACT
> + struct netdev_queue *netdev_queue =
> + rcu_dereference_bh(skb->dev->ingress_queue);
> struct tcf_proto *cl = rcu_dereference_bh(skb->dev->ingress_cl_list);
> struct tcf_result cl_res;
> + struct Qdisc *q;
>
> /* If there's at least one ingress present somewhere (so
> * we get here via enabled static key), remaining devices
> * that are not configured with an ingress qdisc will bail
> * out here.
> */
> - if (!cl)
> + if (!cl || !netdev_queue)
> return skb;
> + q = netdev_queue->qdisc;
> +
> if (*pt_prev) {
> *ret = deliver_skb(skb, *pt_prev, orig_dev);
> *pt_prev = NULL;
> @@ -4205,7 +4214,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
>
> qdisc_skb_cb(skb)->pkt_len = skb->len;
> skb->tc_at_ingress = 1;
> - qdisc_bstats_cpu_update(cl->q, skb);
> + qdisc_bstats_cpu_update(q, skb);
>
> switch (tcf_classify(skb, cl, &cl_res, false)) {
> case TC_ACT_OK:
> @@ -4213,7 +4222,7 @@ sch_handle_ingress(struct sk_buff *skb, struct packet_type **pt_prev, int *ret,
> skb->tc_index = TC_H_MIN(cl_res.classid);
> break;
> case TC_ACT_SHOT:
> - qdisc_qstats_cpu_drop(cl->q);
> + qdisc_qstats_cpu_drop(q);
> kfree_skb(skb);
> return NULL;
> case TC_ACT_STOLEN:
>
^ permalink raw reply
* Re: [PATCH net-next 01/12] bpf: verifier: set reg_type on context accesses in second pass
From: Daniel Borkmann @ 2017-10-12 21:46 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, oss-drivers, alexei.starovoitov
In-Reply-To: <20171012143950.58c1b601@cakuba.netronome.com>
On 10/12/2017 11:39 PM, Jakub Kicinski wrote:
> On Thu, 12 Oct 2017 23:33:21 +0200, Daniel Borkmann wrote:
>> On 10/12/2017 10:56 PM, Jakub Kicinski wrote:
>>> On Thu, 12 Oct 2017 22:43:10 +0200, Daniel Borkmann wrote:
>> [...]
>>>> It would be nice to keep the reg_type setting in one place, meaning
>>>> the callbacks themselves, so we wouldn't need to maintain this in
>>>> multiple places.
>>>
>>> Hm.. I though this was the smallest and simplest change. I could
>>> translate the offsets but that seems wobbly. Or try to consolidate the
>>> call into the same if () branch? Not sure..
>>
>> Different callbacks for post-verification would be good at min as it
>> would allow to keep all the context access info in one place for a
>> given type at least.
>
> Sorry to be clear - you're suggesting adding a new callback to struct
> bpf_verifier_ops, or swapping the struct bpf_verifier_ops for a
> special post-verification one?
Either way is fine by me.
>>> As a bonus info I discovered there is a bug in -net with how things are
>>> converted. We allow arithmetic on context pointers but then only
>>> look at the insn.off in the converter... I'm working on a fix.
>>
>> Ohh well, good catch, indeed! :( Can you also add coverage to the
>> bpf selftests for this?
>
> Will do!
Thanks,
Daniel
^ permalink raw reply
* Re: [RFC 0/3] Adding config get/set to devlink
From: Roopa Prabhu @ 2017-10-12 21:53 UTC (permalink / raw)
To: Florian Fainelli
Cc: David Miller, Jiří Pírko, Steve Lin,
netdev@vger.kernel.org, Jiri Pirko, Michael Chan, linux-pci,
John W. Linville, Andy Gospodarek
In-Reply-To: <21ab4a5d-0b6a-7976-7bf0-acd334f2613f@gmail.com>
On Thu, Oct 12, 2017 at 12:20 PM, Florian Fainelli <f.fainelli@gmail.com> wrote:
> On 10/12/2017 12:06 PM, David Miller wrote:
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Date: Thu, 12 Oct 2017 08:43:59 -0700
>>
>>> Once we move ethtool (or however we name its successor) over to
>>> netlink there is an opportunity for accessing objects that do and do
>>> not have a netdevice representor today (e.g: management ports on
>>> switches) with the same interface, and devlink could be used for
>>> that.
>>
>> That is an interesting angle for including this in devlink.
>>
>> I'm not so sure what to do about this.
>>
>> One suggestion is that devlink is used for getting ethtool stats for
>> objects lacking netdev representor's, and a new genetlink family is
>> used for netdev based ethtool.
>
> Right, I was also thinking along those lines that we we would have a new
> generic netlink family for ethtool to support ethtool over netlink.
new api is fine by me. The reason for suggesting devlink was because
some of the devlink
port_* ops are close to ethtool ops that can operate on a port/netdev.
eg split_port could be a netdev operation
unless you want to split before the netdev is created.
There are some ops in devlink which are global hw parameters and not
specific to a port, those fit perfectly with
devlinks original goal.
>
>>
>> I think it's important that we don't expand the scope of devlink
>> beyond what it was originally designed for.
>
> It seems to me like devlink is well defined in what it is not for: it is
> not meant to be used for any object that is/has a net_device, but it is
> not well defined for what it can offer to these non network devices. For
> instance, we have a tremendous amount of operations that are extremely
> specific to its single user(s) such as mlx5 and mlxsw.
>
> For instance, I am not sure how the buffer reservation scheme can be
> generalized, and this is always the tricky part with a single user
> facility in that you try to generalize the best you can based on the HW
> you know. This is not a criticism or meant to be anything negative, this
> just happens to be the case, and we did not have anything better.
>
> So maybe the first thing is to clarify what devlink operations can and
> should be and what they are absolutely not allowed to cover. We should
> also clarify whether a generic set/get that Steven is proposing is
> something that we tolerate, or whether there should be specific function
> pointers implemented for each attribute, which would be more in line
> with what has been done thus far.
> --
> Florian
^ permalink raw reply
* Re: Ethtool question
From: Roopa Prabhu @ 2017-10-12 22:00 UTC (permalink / raw)
To: Ben Greear; +Cc: David Miller, John W. Linville, netdev@vger.kernel.org
In-Reply-To: <59DFE275.3050805@candelatech.com>
On Thu, Oct 12, 2017 at 2:45 PM, Ben Greear <greearb@candelatech.com> wrote:
> On 10/11/2017 01:49 PM, David Miller wrote:
>>
>> From: "John W. Linville" <linville@tuxdriver.com>
>> Date: Wed, 11 Oct 2017 16:44:07 -0400
>>
>>> On Wed, Oct 11, 2017 at 09:51:56AM -0700, Ben Greear wrote:
>>>>
>>>> I noticed today that setting some ethtool settings to the same value
>>>> returns an error code. I would think this should silently return
>>>> success instead? Makes it easier to call it from scripts this way:
>>>>
>>>> [root@lf0313-6477 lanforge]# ethtool -L eth3 combined 1
>>>> combined unmodified, ignoring
>>>> no channel parameters changed, aborting
>>>> current values: tx 0 rx 0 other 1 combined 1
>>>> [root@lf0313-6477 lanforge]# echo $?
>>>> 1
>>>
>>>
>>> I just had this discussion a couple of months ago with someone. My
>>> initial feeling was like you, a no-op is not a failure. But someone
>>> convinced me otherwise...I will now endeavour to remember who that
>>> was and how they convinced me...
>>>
>>> Anyone else have input here?
>>
>>
>> I guess this usually happens when drivers don't support changing the
>> settings at all. So they just make their ethtool operation for the
>> 'set' always return an error.
>>
>> We could have a generic ethtool helper that does "get" and then if the
>> "set" request is identical just return zero.
>>
>> But from another perspective, the error returned from the "set" in this
>> situation also indicates to the user that the driver does not support
>> the "set" operation which has value and meaning in and of itself. And
>> we'd lose that with the given suggestion.
>
>
> In my case, the driver (igb) does support the set, my program just made the
> same
> ethtool call several times and it fails after the initial change (that
> actually
> changes something), as best as I can figure.
This error is returned by ethtool user-space. It does a get, check and
then set if user has requested changes.
^ permalink raw reply
* Re: [net-next PATCH] mqprio: Reserve last 32 classid values for HW traffic classes and misc IDs
From: Jesus Sanchez-Palencia @ 2017-10-12 21:56 UTC (permalink / raw)
To: Alexander Duyck, jiri, amritha.nambiar, vinicius.gomes, netdev,
jhs, davem
In-Reply-To: <20171012182658.14632.9010.stgit@localhost.localdomain>
Hi Alex,
On 10/12/2017 11:38 AM, Alexander Duyck wrote:
> From: Alexander Duyck <alexander.h.duyck@intel.com>
>
> This patch makes a slight tweak to mqprio in order to bring the
> classid values used back in line with what is used for mq. The general idea
> is to reserve values :ffe0 - :ffef to identify hardware traffic classes
> normally reported via dev->num_tc. By doing this we can maintain a
> consistent behavior with mq for classid where :1 - :ffdf will represent a
> physical qdisc mapped onto a Tx queue represented by classid - 1, and the
> traffic classes will be mapped onto a known subset of classid values
> reserved for our virtual qdiscs.
>
> Note I reserved the range from :fff0 - :ffff since this way we might be
> able to reuse these classid values with clsact and ingress which would mean
> that for mq, mqprio, ingress, and clsact we should be able to maintain a
> similar classid layout.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>
> So I thought I would put this out here as a first step towards trying to
> address some of Jiri's concerns about wanting to have a consistent
> userspace API.
>
> The plan is to follow this up with patches to ingress and clsact to look at
> exposing a set of virtual qdiscs similar to what we already have for the HW
> traffic classes in mqprio, although I won't bother with the ability to dump
> class stats since they don't actually enqueue anything.
>
> include/uapi/linux/pkt_sched.h | 1 +
> net/sched/sch_mqprio.c | 79 +++++++++++++++++++++++-----------------
> 2 files changed, 47 insertions(+), 33 deletions(-)
>
> diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
> index 099bf5528fed..174f1cf7e7f9 100644
> --- a/include/uapi/linux/pkt_sched.h
> +++ b/include/uapi/linux/pkt_sched.h
> @@ -74,6 +74,7 @@ struct tc_estimator {
> #define TC_H_INGRESS (0xFFFFFFF1U)
> #define TC_H_CLSACT TC_H_INGRESS
>
> +#define TC_H_MIN_PRIORITY 0xFFE0U
> #define TC_H_MIN_INGRESS 0xFFF2U
> #define TC_H_MIN_EGRESS 0xFFF3U
>
> diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
> index 6bcdfe6e7b63..a61ef119a556 100644
> --- a/net/sched/sch_mqprio.c
> +++ b/net/sched/sch_mqprio.c
> @@ -115,6 +115,10 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
> if (!netif_is_multiqueue(dev))
> return -EOPNOTSUPP;
>
> + /* make certain can allocate enough classids to handle queues */
> + if (dev->num_tx_queues >= TC_H_MIN_PRIORITY)
> + return -ENOMEM;
> +
> if (!opt || nla_len(opt) < sizeof(*qopt))
> return -EINVAL;
>
> @@ -193,7 +197,7 @@ static struct netdev_queue *mqprio_queue_get(struct Qdisc *sch,
> unsigned long cl)
> {
> struct net_device *dev = qdisc_dev(sch);
> - unsigned long ntx = cl - 1 - netdev_get_num_tc(dev);
> + unsigned long ntx = cl - 1;
>
> if (ntx >= dev->num_tx_queues)
> return NULL;
> @@ -282,38 +286,35 @@ static unsigned long mqprio_find(struct Qdisc *sch, u32 classid)
> struct net_device *dev = qdisc_dev(sch);
> unsigned int ntx = TC_H_MIN(classid);
>
> - if (ntx > dev->num_tx_queues + netdev_get_num_tc(dev))
> - return 0;
> - return ntx;
> + /* There are essentially two regions here that have valid classid
> + * values. The first region will have a classid value of 1 through
> + * num_tx_queues. All of these are backed by actual Qdiscs.
> + */
> + if (ntx < TC_H_MIN_PRIORITY)
> + return (ntx <= dev->num_tx_queues) ? ntx : 0;
> +
> + /* The second region represents the hardware traffic classes. These
> + * are represented by classid values of TC_H_MIN_PRIORITY through
> + * TC_H_MIN_PRIORITY + netdev_get_num_tc - 1
> + */
> + return ((ntx - TC_H_MIN_PRIORITY) < netdev_get_num_tc(dev)) ? ntx : 0;
> }
>
> static int mqprio_dump_class(struct Qdisc *sch, unsigned long cl,
> struct sk_buff *skb, struct tcmsg *tcm)
> {
> - struct net_device *dev = qdisc_dev(sch);
> + if (cl < TC_H_MIN_PRIORITY) {
> + struct netdev_queue *dev_queue = mqprio_queue_get(sch, cl);
> + struct net_device *dev = qdisc_dev(sch);
> + int tc = netdev_txq_to_tc(dev, cl - 1);
>
> - if (cl <= netdev_get_num_tc(dev)) {
> + tcm->tcm_parent = (tc < 0) ? 0 :
> + TC_H_MAKE(TC_H_MAJ(sch->handle),
> + TC_H_MIN(tc + TC_H_MIN_PRIORITY));
> + tcm->tcm_info = dev_queue->qdisc_sleeping->handle;
> + } else {
> tcm->tcm_parent = TC_H_ROOT;
> tcm->tcm_info = 0;
> - } else {
> - int i;
> - struct netdev_queue *dev_queue;
> -
> - dev_queue = mqprio_queue_get(sch, cl);
> - tcm->tcm_parent = 0;
> - for (i = 0; i < netdev_get_num_tc(dev); i++) {
> - struct netdev_tc_txq tc = dev->tc_to_txq[i];
> - int q_idx = cl - netdev_get_num_tc(dev);
> -
> - if (q_idx > tc.offset &&
> - q_idx <= tc.offset + tc.count) {
> - tcm->tcm_parent =
> - TC_H_MAKE(TC_H_MAJ(sch->handle),
> - TC_H_MIN(i + 1));
> - break;
> - }
> - }
> - tcm->tcm_info = dev_queue->qdisc_sleeping->handle;
> }
> tcm->tcm_handle |= TC_H_MIN(cl);
> return 0;
> @@ -324,15 +325,14 @@ static int mqprio_dump_class_stats(struct Qdisc *sch, unsigned long cl,
> __releases(d->lock)
> __acquires(d->lock)
> {
> - struct net_device *dev = qdisc_dev(sch);
> -
> - if (cl <= netdev_get_num_tc(dev)) {
> + if (cl >= TC_H_MIN_PRIORITY) {
> int i;
> __u32 qlen = 0;
> struct Qdisc *qdisc;
> struct gnet_stats_queue qstats = {0};
> struct gnet_stats_basic_packed bstats = {0};
> - struct netdev_tc_txq tc = dev->tc_to_txq[cl - 1];
> + struct net_device *dev = qdisc_dev(sch);
> + struct netdev_tc_txq tc = dev->tc_to_txq[cl & TC_BITMASK];
>
> /* Drop lock here it will be reclaimed before touching
> * statistics this is required because the d->lock we
> @@ -385,12 +385,25 @@ static void mqprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
>
> /* Walk hierarchy with a virtual class per tc */
> arg->count = arg->skip;
> - for (ntx = arg->skip;
> - ntx < dev->num_tx_queues + netdev_get_num_tc(dev);
> - ntx++) {
> + for (ntx = arg->skip; ntx < netdev_get_num_tc(dev); ntx++) {
> + if (arg->fn(sch, ntx + TC_H_MIN_PRIORITY, arg) < 0) {
> + arg->stop = 1;
> + return;
> + }
> + arg->count++;
> + }
> +
> + /* Pad the values and skip over unused traffic classes */
> + if (ntx < TC_MAX_QUEUE) {
> + arg->count = TC_MAX_QUEUE;
> + ntx = TC_MAX_QUEUE;
> + }
> +
> + /* Reset offset, sort out remaining per-queue qdiscs */
> + for (ntx -= TC_MAX_QUEUE; ntx < dev->num_tx_queues; ntx++) {
> if (arg->fn(sch, ntx + 1, arg) < 0) {
> arg->stop = 1;
> - break;
> + return;
> }
> arg->count++;
> }
>
Tested-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Looks good.
thanks,
Jesus
^ permalink raw reply
* [PATCH net-next] net: dsa: set random switch address
From: Vivien Didelot @ 2017-10-12 22:10 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
An Ethernet switch may support having a MAC address, which can be used
as the switch's source address in transmitted full-duplex Pause frames.
If a DSA switch supports the related .set_addr operation, the DSA core
sets the master's MAC address on the switch.
This won't make sense anymore in a multi-CPU ports system, because there
won't be a unique master device assigned to a switch tree.
To fix this, assign a random MAC address to the switch chip instead.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa2.c | 8 +++-----
net/dsa/dsa_priv.h | 1 +
net/dsa/legacy.c | 8 +++-----
net/dsa/switch.c | 17 +++++++++++++++++
4 files changed, 24 insertions(+), 10 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 54ed054777bd..8e5780ddd7f9 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -336,11 +336,9 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
if (err)
return err;
- if (ds->ops->set_addr) {
- err = ds->ops->set_addr(ds, dst->cpu_dp->netdev->dev_addr);
- if (err < 0)
- return err;
- }
+ err = dsa_switch_set_addr(ds);
+ if (err)
+ return err;
if (!ds->slave_mii_bus && ds->ops->phy_read) {
ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 2850077cc9cc..9c4c17a4bd6b 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -172,6 +172,7 @@ void dsa_slave_unregister_notifier(void);
/* switch.c */
int dsa_switch_register_notifier(struct dsa_switch *ds);
void dsa_switch_unregister_notifier(struct dsa_switch *ds);
+int dsa_switch_set_addr(struct dsa_switch *ds);
/* tag_brcm.c */
extern const struct dsa_device_ops brcm_netdev_ops;
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 19ff6e0a21dc..340ca7997271 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -172,11 +172,9 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
if (ret)
return ret;
- if (ops->set_addr) {
- ret = ops->set_addr(ds, master->dev_addr);
- if (ret < 0)
- return ret;
- }
+ ret = dsa_switch_set_addr(ds);
+ if (ret)
+ return ret;
if (!ds->slave_mii_bus && ops->phy_read) {
ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e6c06aa349a6..b45a26b006af 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -10,6 +10,7 @@
* (at your option) any later version.
*/
+#include <linux/etherdevice.h>
#include <linux/netdevice.h>
#include <linux/notifier.h>
#include <net/switchdev.h>
@@ -267,3 +268,19 @@ void dsa_switch_unregister_notifier(struct dsa_switch *ds)
if (err)
dev_err(ds->dev, "failed to unregister notifier (%d)\n", err);
}
+
+int dsa_switch_set_addr(struct dsa_switch *ds)
+{
+ u8 addr[6];
+ int err;
+
+ if (ds->ops->set_addr) {
+ eth_random_addr(addr);
+
+ err = ds->ops->set_addr(ds, addr);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
--
2.14.2
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: set random switch address
From: Florian Fainelli @ 2017-10-12 22:21 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <20171012221009.14615-1-vivien.didelot@savoirfairelinux.com>
On 10/12/2017 03:10 PM, Vivien Didelot wrote:
> An Ethernet switch may support having a MAC address, which can be used
> as the switch's source address in transmitted full-duplex Pause frames.
>
> If a DSA switch supports the related .set_addr operation, the DSA core
> sets the master's MAC address on the switch.
>
> This won't make sense anymore in a multi-CPU ports system, because there
> won't be a unique master device assigned to a switch tree.
Thus far, everything you have said is true, but why we should do it,
that is: what if we don't, needs to be explained. Does that create a
problem with the generation of pause frames throughout the switch fabric?
>
> To fix this, assign a random MAC address to the switch chip instead.
Maybe this is something that should be removed entirely from the DSA
core and pushed into the individual switch drivers instead. dsa_loop
implements it for code coverage, but that does not do anything.
set_addr is confusing in that you may think it could be used to program
the switch with the MAC address of the CPU/management port such that you
can disable MAC address learning on said port, but in fact, that's not
how it is used.
>
> Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
> ---
> net/dsa/dsa2.c | 8 +++-----
> net/dsa/dsa_priv.h | 1 +
> net/dsa/legacy.c | 8 +++-----
> net/dsa/switch.c | 17 +++++++++++++++++
> 4 files changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
> index 54ed054777bd..8e5780ddd7f9 100644
> --- a/net/dsa/dsa2.c
> +++ b/net/dsa/dsa2.c
> @@ -336,11 +336,9 @@ static int dsa_ds_apply(struct dsa_switch_tree *dst, struct dsa_switch *ds)
> if (err)
> return err;
>
> - if (ds->ops->set_addr) {
> - err = ds->ops->set_addr(ds, dst->cpu_dp->netdev->dev_addr);
> - if (err < 0)
> - return err;
> - }
> + err = dsa_switch_set_addr(ds);
> + if (err)
> + return err;
>
> if (!ds->slave_mii_bus && ds->ops->phy_read) {
> ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 2850077cc9cc..9c4c17a4bd6b 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -172,6 +172,7 @@ void dsa_slave_unregister_notifier(void);
> /* switch.c */
> int dsa_switch_register_notifier(struct dsa_switch *ds);
> void dsa_switch_unregister_notifier(struct dsa_switch *ds);
> +int dsa_switch_set_addr(struct dsa_switch *ds);
>
> /* tag_brcm.c */
> extern const struct dsa_device_ops brcm_netdev_ops;
> diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
> index 19ff6e0a21dc..340ca7997271 100644
> --- a/net/dsa/legacy.c
> +++ b/net/dsa/legacy.c
> @@ -172,11 +172,9 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
> if (ret)
> return ret;
>
> - if (ops->set_addr) {
> - ret = ops->set_addr(ds, master->dev_addr);
> - if (ret < 0)
> - return ret;
> - }
> + ret = dsa_switch_set_addr(ds);
> + if (ret)
> + return ret;
>
> if (!ds->slave_mii_bus && ops->phy_read) {
> ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
> diff --git a/net/dsa/switch.c b/net/dsa/switch.c
> index e6c06aa349a6..b45a26b006af 100644
> --- a/net/dsa/switch.c
> +++ b/net/dsa/switch.c
> @@ -10,6 +10,7 @@
> * (at your option) any later version.
> */
>
> +#include <linux/etherdevice.h>
> #include <linux/netdevice.h>
> #include <linux/notifier.h>
> #include <net/switchdev.h>
> @@ -267,3 +268,19 @@ void dsa_switch_unregister_notifier(struct dsa_switch *ds)
> if (err)
> dev_err(ds->dev, "failed to unregister notifier (%d)\n", err);
> }
> +
> +int dsa_switch_set_addr(struct dsa_switch *ds)
> +{
> + u8 addr[6];
> + int err;
> +
> + if (ds->ops->set_addr) {
> + eth_random_addr(addr);
> +
> + err = ds->ops->set_addr(ds, addr);
> + if (err)
> + return err;
> + }
> +
> + return 0;
> +}
>
--
Florian
^ permalink raw reply
* Re: [PATCH] Add -target to clang switches while cross compiling.
From: Alexei Starovoitov @ 2017-10-12 22:23 UTC (permalink / raw)
To: Abhijit Ayarekar; +Cc: ast, daniel, netdev, linux-kernel, yhs
In-Reply-To: <1507841157-10487-1-git-send-email-abhijit.ayarekar@caviumnetworks.com>
On Thu, Oct 12, 2017 at 01:45:57PM -0700, Abhijit Ayarekar wrote:
> Latest llvm update excludes assembly instructions.
> As a result __ASM_SYSREGS_H define is not required.
> -target switch includes appropriate target specific files.
>
> Tested on x86 and arm64 with llvm with git revision
> commit df6ca162269f9d756f8742bf4b658dcf690e3eb5
> Author: Yonghong Song <yhs@fb.com>
> Date: Thu Sep 28 02:46:11 2017 +0000
>
> bpf: add new insns for bswap_to_le and negation
>
> Signed-off-by: Abhijit Ayarekar <abhijit.ayarekar@caviumnetworks.com>
> ---
> samples/bpf/Makefile | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> index ebc2ad6..81f9fcd 100644
> --- a/samples/bpf/Makefile
> +++ b/samples/bpf/Makefile
> @@ -180,6 +180,7 @@ CLANG ?= clang
> # Detect that we're cross compiling and use the cross compiler
> ifdef CROSS_COMPILE
> HOSTCC = $(CROSS_COMPILE)gcc
> +CLANG_ARCH_ARGS = -target $(ARCH)
this is only need because you're crosscompiling, right?
In native compilation it's unnecessary flag.
Only droping -D__ASM_SYSREG_H is enough, correct?
> endif
>
> # Trick to allow make to be run from this directory
> @@ -229,9 +230,9 @@ $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
> $(obj)/%.o: $(src)/%.c
> $(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> -I$(srctree)/tools/testing/selftests/bpf/ \
> - -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> + -D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
> -D__TARGET_ARCH_$(ARCH) -Wno-compare-distinct-pointer-types \
> -Wno-gnu-variable-sized-type-not-at-end \
> -Wno-address-of-packed-member -Wno-tautological-compare \
> - -Wno-unknown-warning-option \
> + -Wno-unknown-warning-option $(CLANG_ARCH_ARGS) \
> -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: set random switch address
From: Vivien Didelot @ 2017-10-12 22:35 UTC (permalink / raw)
To: Florian Fainelli, netdev
Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <b9aecb73-b240-7b59-4315-ea87868396cd@gmail.com>
Hi Florian,
Florian Fainelli <f.fainelli@gmail.com> writes:
> On 10/12/2017 03:10 PM, Vivien Didelot wrote:
>> An Ethernet switch may support having a MAC address, which can be used
>> as the switch's source address in transmitted full-duplex Pause frames.
>>
>> If a DSA switch supports the related .set_addr operation, the DSA core
>> sets the master's MAC address on the switch.
>>
>> This won't make sense anymore in a multi-CPU ports system, because there
>> won't be a unique master device assigned to a switch tree.
>
> Thus far, everything you have said is true, but why we should do it,
> that is: what if we don't, needs to be explained. Does that create a
> problem with the generation of pause frames throughout the switch fabric?
>
>>
>> To fix this, assign a random MAC address to the switch chip instead.
>
> Maybe this is something that should be removed entirely from the DSA
> core and pushed into the individual switch drivers instead. dsa_loop
> implements it for code coverage, but that does not do anything.
>
> set_addr is confusing in that you may think it could be used to program
> the switch with the MAC address of the CPU/management port such that you
> can disable MAC address learning on said port, but in fact, that's not
> how it is used.
You are correct. So what I can do is assign a random MAC address in the
Marvell driver, remove the .set_addr implementation of mv88e6xxx and
dsa_loop, and finally remove this code from DSA core completely.
Thanks,
Vivien
^ permalink raw reply
* [net-next RFC 0/4] Openvswitch meter action
From: Andy Zhou @ 2017-10-12 22:38 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
This patch series is the first attempt to add openvswitch
meter support. We have previously experimented with adding
metering support in nftables. However 1) It was not clear
how to expose a named nftables object cleanly, and 2)
the logic that implements metering is quite small, < 100 lines
of code.
With those two observations, it seems cleaner to add meter
support in the openvswitch module directly.
Andy Zhou (4):
openvswitch: Add meter netlink definitions
openvswitch: export get_dp() API.
openvswitch: Add meter infrastructure
openvswitch: Add meter action support
include/uapi/linux/openvswitch.h | 52 ++++
net/openvswitch/Makefile | 1 +
net/openvswitch/actions.c | 12 +
net/openvswitch/datapath.c | 43 +--
net/openvswitch/datapath.h | 35 +++
net/openvswitch/flow_netlink.c | 6 +
net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++
net/openvswitch/meter.h | 54 ++++
8 files changed, 783 insertions(+), 31 deletions(-)
create mode 100644 net/openvswitch/meter.c
create mode 100644 net/openvswitch/meter.h
--
1.8.3.1
^ permalink raw reply
* [PATCH] tracing: bpf: Hide bpf trace events when they are not used
From: Steven Rostedt @ 2017-10-12 22:40 UTC (permalink / raw)
To: LKML; +Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller, netdev
From: Steven Rostedt (VMware) <rostedt@goodmis.org>
All the trace events defined in include/trace/events/bpf.h are only
used when CONFIG_BPF_SYSCALL is defined. But this file gets included by
include/linux/bpf_trace.h which is included by the networking code with
CREATE_TRACE_POINTS defined.
If a trace event is created but not used it still has data structures
and functions created for its use, even though nothing is using them.
To not waste space, do not define the BPF trace events in bpf.h unless
CONFIG_BPF_SYSCALL is defined.
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
Index: linux-trace.git/include/trace/events/bpf.h
===================================================================
--- linux-trace.git.orig/include/trace/events/bpf.h
+++ linux-trace.git/include/trace/events/bpf.h
@@ -4,6 +4,9 @@
#if !defined(_TRACE_BPF_H) || defined(TRACE_HEADER_MULTI_READ)
#define _TRACE_BPF_H
+/* These are only used within the BPF_SYSCALL code */
+#ifdef CONFIG_BPF_SYSCALL
+
#include <linux/filter.h>
#include <linux/bpf.h>
#include <linux/fs.h>
@@ -345,7 +348,7 @@ TRACE_EVENT(bpf_map_next_key,
__print_hex(__get_dynamic_array(nxt), __entry->key_len),
__entry->key_trunc ? " ..." : "")
);
-
+#endif /* CONFIG_BPF_SYSCALL */
#endif /* _TRACE_BPF_H */
#include <trace/define_trace.h>
Index: linux-trace.git/kernel/bpf/core.c
===================================================================
--- linux-trace.git.orig/kernel/bpf/core.c
+++ linux-trace.git/kernel/bpf/core.c
@@ -1498,5 +1498,8 @@ int __weak skb_copy_bits(const struct sk
EXPORT_TRACEPOINT_SYMBOL_GPL(xdp_exception);
+/* These are only used within the BPF_SYSCALL code */
+#ifdef CONFIG_BPF_SYSCALL
EXPORT_TRACEPOINT_SYMBOL_GPL(bpf_prog_get_type);
EXPORT_TRACEPOINT_SYMBOL_GPL(bpf_prog_put_rcu);
+#endif
^ permalink raw reply
* [net-next RFC 1/4] openvswitch: Add meter netlink definitions
From: Andy Zhou @ 2017-10-12 22:38 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1507847923-13612-1-git-send-email-azhou@ovn.org>
Meter has its own netlink family. Define netlink messages and attributes
for communicating with the user space programs.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
include/uapi/linux/openvswitch.h | 51 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 156ee4cab82e..325049a129e4 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -848,4 +848,55 @@ enum ovs_action_attr {
#define OVS_ACTION_ATTR_MAX (__OVS_ACTION_ATTR_MAX - 1)
+/* Meters. */
+#define OVS_METER_FAMILY "ovs_meter"
+#define OVS_METER_MCGROUP "ovs_meter"
+#define OVS_METER_VERSION 0x1
+
+enum ovs_meter_cmd {
+ OVS_METER_CMD_UNSPEC,
+ OVS_METER_CMD_FEATURES, /* Get features supported by the datapath. */
+ OVS_METER_CMD_SET, /* Add or modify a meter. */
+ OVS_METER_CMD_DEL, /* Delete a meter. */
+ OVS_METER_CMD_GET /* Get meter stats. */
+};
+
+enum ovs_meter_attr {
+ OVS_METER_ATTR_UNSPEC,
+ OVS_METER_ATTR_ID, /* u32 meter ID within datapath. */
+ OVS_METER_ATTR_KBPS, /* No argument. If set, units in kilobits
+ * per second. Otherwise, units in
+ * packets per second.
+ */
+ OVS_METER_ATTR_STATS, /* struct ovs_flow_stats for the meter. */
+ OVS_METER_ATTR_BANDS, /* Nested attributes for meter bands. */
+ OVS_METER_ATTR_USED, /* u64 msecs last used in monotonic time. */
+ OVS_METER_ATTR_CLEAR, /* Flag to clear stats, used. */
+ OVS_METER_ATTR_MAX_METERS, /* u32 number of meters supported. */
+ OVS_METER_ATTR_MAX_BANDS, /* u32 max number of bands per meter. */
+ OVS_METER_ATTR_PAD,
+ __OVS_METER_ATTR_MAX
+};
+
+#define OVS_METER_ATTR_MAX (__OVS_METER_ATTR_MAX - 1)
+
+enum ovs_band_attr {
+ OVS_BAND_ATTR_UNSPEC,
+ OVS_BAND_ATTR_TYPE, /* u32 OVS_METER_BAND_TYPE_* constant. */
+ OVS_BAND_ATTR_RATE, /* u32 band rate in meter units (see above). */
+ OVS_BAND_ATTR_BURST, /* u32 burst size in meter units. */
+ OVS_BAND_ATTR_STATS, /* struct ovs_flow_stats for the band. */
+ __OVS_BAND_ATTR_MAX
+};
+
+#define OVS_BAND_ATTR_MAX (__OVS_BAND_ATTR_MAX - 1)
+
+enum ovs_meter_band_type {
+ OVS_METER_BAND_TYPE_UNSPEC,
+ OVS_METER_BAND_TYPE_DROP, /* Drop exceeding packets. */
+ __OVS_METER_BAND_TYPE_MAX
+};
+
+#define OVS_METER_BAND_TYPE_MAX (__OVS_METER_BAND_TYPE_MAX - 1)
+
#endif /* _LINUX_OPENVSWITCH_H */
--
1.8.3.1
^ permalink raw reply related
* [net-next RFC 2/4] openvswitch: export get_dp() API.
From: Andy Zhou @ 2017-10-12 22:38 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1507847923-13612-1-git-send-email-azhou@ovn.org>
Later patches will invoke get_dp() outside of datapath.c. Export it.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
net/openvswitch/datapath.c | 29 -----------------------------
net/openvswitch/datapath.h | 31 +++++++++++++++++++++++++++++++
2 files changed, 31 insertions(+), 29 deletions(-)
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index c3aec6227c91..ac7154018676 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -142,35 +142,6 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *,
const struct dp_upcall_info *,
uint32_t cutlen);
-/* Must be called with rcu_read_lock. */
-static struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
-{
- struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
-
- if (dev) {
- struct vport *vport = ovs_internal_dev_get_vport(dev);
- if (vport)
- return vport->dp;
- }
-
- return NULL;
-}
-
-/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
- * returned dp pointer valid.
- */
-static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
-{
- struct datapath *dp;
-
- WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
- rcu_read_lock();
- dp = get_dp_rcu(net, dp_ifindex);
- rcu_read_unlock();
-
- return dp;
-}
-
/* Must be called with rcu_read_lock or ovs_mutex. */
const char *ovs_dp_name(const struct datapath *dp)
{
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index 480600649d0b..ad14b571219d 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -30,6 +30,7 @@
#include "conntrack.h"
#include "flow.h"
#include "flow_table.h"
+#include "vport-internal_dev.h"
#define DP_MAX_PORTS USHRT_MAX
#define DP_VPORT_HASH_BUCKETS 1024
@@ -190,6 +191,36 @@ static inline struct vport *ovs_vport_ovsl(const struct datapath *dp, int port_n
return ovs_lookup_vport(dp, port_no);
}
+/* Must be called with rcu_read_lock. */
+static inline struct datapath *get_dp_rcu(struct net *net, int dp_ifindex)
+{
+ struct net_device *dev = dev_get_by_index_rcu(net, dp_ifindex);
+
+ if (dev) {
+ struct vport *vport = ovs_internal_dev_get_vport(dev);
+
+ if (vport)
+ return vport->dp;
+ }
+
+ return NULL;
+}
+
+/* The caller must hold either ovs_mutex or rcu_read_lock to keep the
+ * returned dp pointer valid.
+ */
+static inline struct datapath *get_dp(struct net *net, int dp_ifindex)
+{
+ struct datapath *dp;
+
+ WARN_ON_ONCE(!rcu_read_lock_held() && !lockdep_ovsl_is_held());
+ rcu_read_lock();
+ dp = get_dp_rcu(net, dp_ifindex);
+ rcu_read_unlock();
+
+ return dp;
+}
+
extern struct notifier_block ovs_dp_device_notifier;
extern struct genl_family dp_vport_genl_family;
--
1.8.3.1
^ permalink raw reply related
* [net-next RFC 3/4] openvswitch: Add meter infrastructure
From: Andy Zhou @ 2017-10-12 22:38 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1507847923-13612-1-git-send-email-azhou@ovn.org>
OVS kernel datapath so far does not support Openflow meter action.
This is the first stab at adding kernel datapath meter support.
This implementation supports only drop band type.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
net/openvswitch/Makefile | 1 +
net/openvswitch/datapath.c | 14 +-
net/openvswitch/datapath.h | 3 +
net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++++++++
net/openvswitch/meter.h | 54 ++++
5 files changed, 681 insertions(+), 2 deletions(-)
create mode 100644 net/openvswitch/meter.c
create mode 100644 net/openvswitch/meter.h
diff --git a/net/openvswitch/Makefile b/net/openvswitch/Makefile
index 60f809085b92..658383fbdf53 100644
--- a/net/openvswitch/Makefile
+++ b/net/openvswitch/Makefile
@@ -11,6 +11,7 @@ openvswitch-y := \
flow.o \
flow_netlink.o \
flow_table.o \
+ meter.o \
vport.o \
vport-internal_dev.o \
vport-netdev.o
diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
index ac7154018676..eef8d3ea3aae 100644
--- a/net/openvswitch/datapath.c
+++ b/net/openvswitch/datapath.c
@@ -55,6 +55,7 @@
#include "flow.h"
#include "flow_table.h"
#include "flow_netlink.h"
+#include "meter.h"
#include "vport-internal_dev.h"
#include "vport-netdev.h"
@@ -174,6 +175,7 @@ static void destroy_dp_rcu(struct rcu_head *rcu)
ovs_flow_tbl_destroy(&dp->table);
free_percpu(dp->stats_percpu);
kfree(dp->ports);
+ ovs_meters_exit(dp);
kfree(dp);
}
@@ -1572,6 +1574,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
for (i = 0; i < DP_VPORT_HASH_BUCKETS; i++)
INIT_HLIST_HEAD(&dp->ports[i]);
+ err = ovs_meters_init(dp);
+ if (err)
+ goto err_destroy_ports_array;
+
/* Set up our datapath device. */
parms.name = nla_data(a[OVS_DP_ATTR_NAME]);
parms.type = OVS_VPORT_TYPE_INTERNAL;
@@ -1600,7 +1606,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
ovs_dp_reset_user_features(skb, info);
}
- goto err_destroy_ports_array;
+ goto err_destroy_meters;
}
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
@@ -1615,8 +1621,10 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
ovs_notify(&dp_datapath_genl_family, reply, info);
return 0;
-err_destroy_ports_array:
+err_destroy_meters:
ovs_unlock();
+ ovs_meters_exit(dp);
+err_destroy_ports_array:
kfree(dp->ports);
err_destroy_percpu:
free_percpu(dp->stats_percpu);
@@ -2244,6 +2252,7 @@ struct genl_family dp_vport_genl_family __ro_after_init = {
&dp_vport_genl_family,
&dp_flow_genl_family,
&dp_packet_genl_family,
+ &dp_meter_genl_family,
};
static void dp_unregister_genl(int n_families)
@@ -2424,3 +2433,4 @@ static void dp_cleanup(void)
MODULE_ALIAS_GENL_FAMILY(OVS_VPORT_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_FLOW_FAMILY);
MODULE_ALIAS_GENL_FAMILY(OVS_PACKET_FAMILY);
+MODULE_ALIAS_GENL_FAMILY(OVS_METER_FAMILY);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index ad14b571219d..d1ffa1d9fe57 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -92,6 +92,9 @@ struct datapath {
u32 user_features;
u32 max_headroom;
+
+ /* Switch meters. */
+ struct hlist_head *meters;
};
/**
diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
new file mode 100644
index 000000000000..f24ebb5f7af4
--- /dev/null
+++ b/net/openvswitch/meter.c
@@ -0,0 +1,611 @@
+/*
+ * Copyright (c) 2017 Nicira, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/if.h>
+#include <linux/skbuff.h>
+#include <linux/ip.h>
+#include <linux/kernel.h>
+#include <linux/openvswitch.h>
+#include <linux/netlink.h>
+#include <linux/rculist.h>
+
+#include <net/netlink.h>
+#include <net/genetlink.h>
+
+#include "datapath.h"
+#include "meter.h"
+
+#define METER_HASH_BUCKETS 1024
+
+static const struct nla_policy meter_policy[OVS_METER_ATTR_MAX + 1] = {
+ [OVS_METER_ATTR_ID] = { .type = NLA_U32, },
+ [OVS_METER_ATTR_KBPS] = { .type = NLA_FLAG },
+ [OVS_METER_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
+ [OVS_METER_ATTR_BANDS] = { .type = NLA_NESTED },
+ [OVS_METER_ATTR_USED] = { .type = NLA_U64 },
+ [OVS_METER_ATTR_CLEAR] = { .type = NLA_FLAG },
+ [OVS_METER_ATTR_MAX_METERS] = { .type = NLA_U32 },
+ [OVS_METER_ATTR_MAX_BANDS] = { .type = NLA_U32 },
+};
+
+static const struct nla_policy band_policy[OVS_BAND_ATTR_MAX + 1] = {
+ [OVS_BAND_ATTR_TYPE] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_RATE] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_BURST] = { .type = NLA_U32, },
+ [OVS_BAND_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
+};
+
+static void rcu_free_ovs_meter_callback(struct rcu_head *rcu)
+{
+ struct dp_meter *meter = container_of(rcu, struct dp_meter, rcu);
+
+ kfree(meter);
+}
+
+static void ovs_meter_free(struct dp_meter *meter)
+{
+ if (!meter)
+ return;
+
+ call_rcu(&meter->rcu, rcu_free_ovs_meter_callback);
+}
+
+static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
+ u32 meter_id)
+{
+ return &dp->meters[meter_id & (METER_HASH_BUCKETS - 1)];
+}
+
+/* Call with ovs_mutex or RCU read lock. */
+static struct dp_meter *lookup_meter(const struct datapath *dp,
+ u32 meter_id)
+{
+ struct dp_meter *meter;
+ struct hlist_head *head;
+
+ head = meter_hash_bucket(dp, meter_id);
+ hlist_for_each_entry_rcu(meter, head, dp_hash_node) {
+ if (meter->id == meter_id)
+ return meter;
+ }
+ return NULL;
+}
+
+static void attach_meter(struct datapath *dp, struct dp_meter *meter)
+{
+ struct hlist_head *head = meter_hash_bucket(dp, meter->id);
+
+ hlist_add_head_rcu(&meter->dp_hash_node, head);
+}
+
+static void detach_meter(struct dp_meter *meter)
+{
+ ASSERT_OVSL();
+ if (meter)
+ hlist_del_rcu(&meter->dp_hash_node);
+}
+
+static struct sk_buff *
+ovs_meter_cmd_reply_start(struct genl_info *info, u8 cmd,
+ struct ovs_header **ovs_reply_header)
+{
+ struct sk_buff *skb;
+ struct ovs_header *ovs_header = info->userhdr;
+
+ skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+ if (!skb)
+ return ERR_PTR(-ENOMEM);
+
+ *ovs_reply_header = genlmsg_put(skb, info->snd_portid,
+ info->snd_seq,
+ &dp_meter_genl_family, 0, cmd);
+ if (!ovs_reply_header) {
+ nlmsg_free(skb);
+ return ERR_PTR(-EMSGSIZE);
+ }
+ (*ovs_reply_header)->dp_ifindex = ovs_header->dp_ifindex;
+
+ return skb;
+}
+
+static int ovs_meter_cmd_reply_stats(struct sk_buff *reply, u32 meter_id,
+ struct dp_meter *meter)
+{
+ struct nlattr *nla;
+ struct dp_meter_band *band;
+ u16 i;
+
+ if (nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id))
+ goto error;
+
+ if (!meter)
+ return 0;
+
+ if (nla_put(reply, OVS_METER_ATTR_STATS,
+ sizeof(struct ovs_flow_stats), &meter->stats) ||
+ nla_put_u64_64bit(reply, OVS_METER_ATTR_USED, meter->used,
+ OVS_METER_ATTR_PAD))
+ goto error;
+
+ nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
+ if (!nla)
+ goto error;
+
+ band = meter->bands;
+
+ for (i = 0; i < meter->n_bands; ++i, ++band) {
+ struct nlattr *band_nla;
+
+ band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
+ if (!band_nla || nla_put(reply, OVS_BAND_ATTR_STATS,
+ sizeof(struct ovs_flow_stats),
+ &band->stats))
+ goto error;
+ nla_nest_end(reply, band_nla);
+ }
+ nla_nest_end(reply, nla);
+
+ return 0;
+error:
+ return -EMSGSIZE;
+}
+
+static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
+{
+ struct datapath *dp;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct sk_buff *reply;
+ struct ovs_header *ovs_reply_header;
+ struct nlattr *nla, *band_nla;
+ int err;
+
+ /* Check that the datapath exists */
+ ovs_lock();
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ ovs_unlock();
+ if (!dp)
+ return -ENODEV;
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
+ &ovs_reply_header);
+ if (!reply)
+ return PTR_ERR(reply);
+
+ if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
+ nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
+ goto nla_put_failure;
+
+ nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
+ if (!nla)
+ goto nla_put_failure;
+
+ band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
+ if (!band_nla)
+ goto nla_put_failure;
+ /* Currently only DROP band type is supported. */
+ if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
+ goto nla_put_failure;
+ nla_nest_end(reply, band_nla);
+ nla_nest_end(reply, nla);
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+nla_put_failure:
+ nlmsg_free(reply);
+ err = -EMSGSIZE;
+ return err;
+}
+
+static struct dp_meter *dp_meter_create(struct nlattr **a)
+{
+ struct nlattr *nla;
+ int rem;
+ u16 n_bands = 0;
+ struct dp_meter *meter;
+ struct dp_meter_band *band;
+ int err;
+
+ /* Validate attributes, count the bands. */
+ if (!a[OVS_METER_ATTR_BANDS])
+ return ERR_PTR(-EINVAL);
+
+ nla_for_each_nested(nla, a[OVS_METER_ATTR_BANDS], rem)
+ if (++n_bands > DP_MAX_BANDS)
+ return ERR_PTR(-EINVAL);
+
+ /* Allocate and set up the meter before locking anything. */
+ meter = kzalloc(n_bands * sizeof(struct dp_meter_band) +
+ sizeof(*meter), GFP_KERNEL);
+ if (!meter)
+ return ERR_PTR(-ENOMEM);
+
+ meter->used = ktime_get_ns() / 1000 / 1000;
+ meter->kbps = a[OVS_METER_ATTR_KBPS] ? 1 : 0;
+ meter->keep_stats = !a[OVS_METER_ATTR_CLEAR];
+ spin_lock_init(&meter->lock);
+ if (meter->keep_stats && a[OVS_METER_ATTR_STATS]) {
+ meter->stats = *(struct ovs_flow_stats *)
+ nla_data(a[OVS_METER_ATTR_STATS]);
+ }
+ meter->n_bands = n_bands;
+
+ /* Set up meter bands. */
+ band = meter->bands;
+ nla_for_each_nested(nla, a[OVS_METER_ATTR_BANDS], rem) {
+ struct nlattr *attr[OVS_BAND_ATTR_MAX + 1];
+ u32 band_max_delta_t;
+
+ err = nla_parse((struct nlattr **)&attr, OVS_BAND_ATTR_MAX,
+ nla_data(nla), nla_len(nla), band_policy,
+ NULL);
+ if (err)
+ goto exit_free_meter;
+
+ if (!attr[OVS_BAND_ATTR_TYPE] ||
+ !attr[OVS_BAND_ATTR_RATE] ||
+ !attr[OVS_BAND_ATTR_BURST]) {
+ err = -EINVAL;
+ goto exit_free_meter;
+ }
+
+ band->type = nla_get_u32(attr[OVS_BAND_ATTR_TYPE]);
+ band->rate = nla_get_u32(attr[OVS_BAND_ATTR_RATE]);
+ band->burst_size = nla_get_u32(attr[OVS_BAND_ATTR_BURST]);
+ /* Figure out max delta_t that is enough to fill any bucket.
+ * Keep max_delta_t size to the bucket units:
+ * pkts => 1/1000 packets, kilobits => bits.
+ */
+ band_max_delta_t = (band->burst_size + band->rate) * 1000;
+ /* Start with a full bucket. */
+ band->bucket = band_max_delta_t;
+ if (band_max_delta_t > meter->max_delta_t)
+ meter->max_delta_t = band_max_delta_t;
+ band++;
+ }
+
+ return meter;
+
+exit_free_meter:
+ kfree(meter);
+ return ERR_PTR(err);
+}
+
+static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ struct dp_meter *meter, *old_meter;
+ struct sk_buff *reply;
+ struct ovs_header *ovs_reply_header;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct datapath *dp;
+ int err;
+ u32 meter_id;
+ bool failed;
+
+ meter = dp_meter_create(a);
+ if (IS_ERR_OR_NULL(meter))
+ return PTR_ERR(meter);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
+ &ovs_reply_header);
+ if (IS_ERR(reply)) {
+ err = PTR_ERR(reply);
+ goto exit_free_meter;
+ }
+
+ ovs_lock();
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ if (!a[OVS_METER_ATTR_ID]) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ /* Cannot fail after this. */
+ old_meter = lookup_meter(dp, meter_id);
+ attach_meter(dp, meter);
+ ovs_unlock();
+
+ /* Build response with the meter_id and stats from
+ * the old meter, if any.
+ */
+ failed = nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id);
+ WARN_ON(failed);
+ if (old_meter) {
+ spin_lock_bh(&old_meter->lock);
+ if (old_meter->keep_stats) {
+ err = ovs_meter_cmd_reply_stats(reply, meter_id,
+ old_meter);
+ WARN_ON(err);
+ }
+ spin_unlock_bh(&old_meter->lock);
+ ovs_meter_free(old_meter);
+ }
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+exit_free_meter:
+ kfree(meter);
+ return err;
+}
+
+static int ovs_meter_cmd_get(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ u32 meter_id;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct ovs_header *ovs_reply_header;
+ struct datapath *dp;
+ int err;
+ struct sk_buff *reply;
+ struct dp_meter *meter;
+
+ if (!a[OVS_METER_ATTR_ID])
+ return -EINVAL;
+
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_GET,
+ &ovs_reply_header);
+ if (IS_ERR(reply))
+ return PTR_ERR(reply);
+
+ ovs_lock();
+
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ /* Locate meter, copy stats. */
+ meter = lookup_meter(dp, meter_id);
+ if (!meter) {
+ err = -ENOENT;
+ goto exit_unlock;
+ }
+
+ spin_lock_bh(&meter->lock);
+ err = ovs_meter_cmd_reply_stats(reply, meter_id, meter);
+ spin_unlock_bh(&meter->lock);
+ if (err)
+ goto exit_unlock;
+
+ ovs_unlock();
+
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+ return err;
+}
+
+static int ovs_meter_cmd_del(struct sk_buff *skb, struct genl_info *info)
+{
+ struct nlattr **a = info->attrs;
+ u32 meter_id;
+ struct ovs_header *ovs_header = info->userhdr;
+ struct ovs_header *ovs_reply_header;
+ struct datapath *dp;
+ int err;
+ struct sk_buff *reply;
+ struct dp_meter *old_meter;
+
+ if (!a[OVS_METER_ATTR_ID])
+ return -EINVAL;
+ meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
+
+ reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_DEL,
+ &ovs_reply_header);
+ if (IS_ERR(reply))
+ return PTR_ERR(reply);
+
+ ovs_lock();
+
+ dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
+ if (!dp) {
+ err = -ENODEV;
+ goto exit_unlock;
+ }
+
+ old_meter = lookup_meter(dp, meter_id);
+ if (old_meter) {
+ spin_lock_bh(&old_meter->lock);
+ err = ovs_meter_cmd_reply_stats(reply, meter_id, old_meter);
+ WARN_ON(err);
+ spin_unlock_bh(&old_meter->lock);
+ detach_meter(old_meter);
+ }
+ ovs_unlock();
+ ovs_meter_free(old_meter);
+ genlmsg_end(reply, ovs_reply_header);
+ return genlmsg_reply(reply, info);
+
+exit_unlock:
+ ovs_unlock();
+ nlmsg_free(reply);
+ return err;
+}
+
+/* Meter action execution.
+ *
+ * Return true 'meter_id' drop band is triggered. The 'skb' should be
+ * dropped by the caller'.
+ */
+bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_key *key, u32 meter_id)
+{
+ struct dp_meter *meter;
+ struct dp_meter_band *band;
+ long long int now_ms = ktime_get_ns() / 1000 / 1000;
+ long long int long_delta_ms;
+ u32 delta_ms;
+ u32 cost;
+ int i, band_exceeded_max = -1;
+ u32 band_exceeded_rate = 0;
+
+ meter = lookup_meter(dp, meter_id);
+ /* Do not drop the packet when there is no meter. */
+ if (!meter)
+ return false;
+
+ /* Lock the meter while using it. */
+ spin_lock(&meter->lock);
+
+ long_delta_ms = (now_ms - meter->used); /* ms */
+
+ /* Make sure delta_ms will not be too large, so that bucket will not
+ * wrap around below.
+ */
+ delta_ms = (long_delta_ms > (long long int)meter->max_delta_t)
+ ? meter->max_delta_t : (u32)long_delta_ms;
+
+ /* Update meter statistics.
+ */
+ meter->used = now_ms;
+ meter->stats.n_packets += 1;
+ meter->stats.n_bytes += skb->len;
+
+ /* Bucket rate is either in kilobits per second, or in packets per
+ * second. We maintain the bucket in the units of either bits or
+ * 1/1000th of a packet, correspondingly.
+ * Then, when rate is multiplied with milliseconds, we get the
+ * bucket units:
+ * msec * kbps = bits, and
+ * msec * packets/sec = 1/1000 packets.
+ *
+ * 'cost' is the number of bucket units in this packet.
+ */
+ cost = (meter->kbps) ? skb->len * 8 : 1000;
+
+ /* Update all bands and find the one hit with the highest rate. */
+ for (i = 0; i < meter->n_bands; ++i) {
+ long long int max_bucket_size;
+
+ band = &meter->bands[i];
+ max_bucket_size = (band->burst_size + band->rate) * 1000;
+
+ band->bucket += delta_ms * band->rate;
+ if (band->bucket > max_bucket_size)
+ band->bucket = max_bucket_size;
+
+ if (band->bucket >= cost) {
+ band->bucket -= cost;
+ } else if (band->rate > band_exceeded_rate) {
+ band_exceeded_rate = band->rate;
+ band_exceeded_max = i;
+ }
+ }
+
+ spin_unlock(&meter->lock);
+
+ if (band_exceeded_max >= 0) {
+ /* Update band statistics. */
+ band = &meter->bands[band_exceeded_max];
+ band->stats.n_packets += 1;
+ band->stats.n_bytes += skb->len;
+
+ /* Drop band triggered, let the caller drop the 'skb'. */
+ if (band->type == OVS_METER_BAND_TYPE_DROP)
+ return true;
+ }
+
+ return false;
+}
+
+static struct genl_ops dp_meter_genl_ops[] = {
+ { .cmd = OVS_METER_CMD_FEATURES,
+ .flags = 0, /* OK for unprivileged users. */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_features
+ },
+ { .cmd = OVS_METER_CMD_SET,
+ .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_set,
+ },
+ { .cmd = OVS_METER_CMD_GET,
+ .flags = 0, /* OK for unprivileged users. */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_get,
+ },
+ { .cmd = OVS_METER_CMD_DEL,
+ .flags = GENL_ADMIN_PERM, /* Requires CAP_NET_ADMIN
+ * privilege.
+ */
+ .policy = meter_policy,
+ .doit = ovs_meter_cmd_del
+ },
+};
+
+static const struct genl_multicast_group ovs_meter_multicast_group = {
+ .name = OVS_METER_MCGROUP,
+};
+
+struct genl_family dp_meter_genl_family __ro_after_init = {
+ .hdrsize = sizeof(struct ovs_header),
+ .name = OVS_METER_FAMILY,
+ .version = OVS_METER_VERSION,
+ .maxattr = OVS_METER_ATTR_MAX,
+ .netnsok = true,
+ .parallel_ops = true,
+ .ops = dp_meter_genl_ops,
+ .n_ops = ARRAY_SIZE(dp_meter_genl_ops),
+ .mcgrps = &ovs_meter_multicast_group,
+ .n_mcgrps = 1,
+ .module = THIS_MODULE,
+};
+
+int ovs_meters_init(struct datapath *dp)
+{
+ int i;
+
+ dp->meters = kmalloc_array(METER_HASH_BUCKETS,
+ sizeof(struct hlist_head), GFP_KERNEL);
+
+ if (!dp->meters)
+ return -ENOMEM;
+
+ for (i = 0; i < METER_HASH_BUCKETS; i++)
+ INIT_HLIST_HEAD(&dp->meters[i]);
+
+ return 0;
+}
+
+void ovs_meters_exit(struct datapath *dp)
+{
+ int i;
+
+ for (i = 0; i < METER_HASH_BUCKETS; i++) {
+ struct hlist_head *head = &dp->meters[i];
+ struct dp_meter *meter;
+ struct hlist_node *n;
+
+ hlist_for_each_entry_safe(meter, n, head, dp_hash_node)
+ kfree(meter);
+ }
+
+ kfree(dp->meters);
+}
diff --git a/net/openvswitch/meter.h b/net/openvswitch/meter.h
new file mode 100644
index 000000000000..964ace2650f8
--- /dev/null
+++ b/net/openvswitch/meter.h
@@ -0,0 +1,54 @@
+/*
+ * Copyright (c) 2017 Nicira, Inc.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+
+#ifndef METER_H
+#define METER_H 1
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/netlink.h>
+#include <linux/openvswitch.h>
+#include <linux/genetlink.h>
+#include <linux/skbuff.h>
+
+#include "flow.h"
+struct datapath;
+
+#define DP_MAX_BANDS 1
+
+struct dp_meter_band {
+ u32 type;
+ u32 rate;
+ u32 burst_size;
+ u32 bucket; /* 1/1000 packets, or in bits */
+ struct ovs_flow_stats stats;
+};
+
+struct dp_meter {
+ spinlock_t lock; /* Per meter lock */
+ struct rcu_head rcu;
+ struct hlist_node dp_hash_node; /*Element in datapath->meters
+ * hash table.
+ */
+ u32 id;
+ u16 kbps:1, keep_stats:1;
+ u16 n_bands;
+ u32 max_delta_t;
+ u64 used;
+ struct ovs_flow_stats stats;
+ struct dp_meter_band bands[];
+};
+
+extern struct genl_family dp_meter_genl_family;
+int ovs_meters_init(struct datapath *dp);
+void ovs_meters_exit(struct datapath *dp);
+bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
+ struct sw_flow_key *key, u32 meter_id);
+
+#endif /* meter.h */
--
1.8.3.1
^ permalink raw reply related
* [net-next RFC 4/4] openvswitch: Add meter action support
From: Andy Zhou @ 2017-10-12 22:38 UTC (permalink / raw)
To: netdev; +Cc: pshelar, joe, gvrose8192, Andy Zhou
In-Reply-To: <1507847923-13612-1-git-send-email-azhou@ovn.org>
Implements OVS kernel meter action support.
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
include/uapi/linux/openvswitch.h | 1 +
net/openvswitch/actions.c | 12 ++++++++++++
net/openvswitch/datapath.h | 1 +
net/openvswitch/flow_netlink.c | 6 ++++++
4 files changed, 20 insertions(+)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 325049a129e4..11fe1a06cdd6 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -835,6 +835,7 @@ enum ovs_action_attr {
OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
OVS_ACTION_ATTR_POP_ETH, /* No argument. */
+ OVS_ACTION_ATTR_METER, /* u32 meter ID. */
__OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
* from userspace. */
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index a54a556fcdb5..4eb160ac5a27 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -1210,6 +1210,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
case OVS_ACTION_ATTR_POP_ETH:
err = pop_eth(skb, key);
break;
+
+ case OVS_ACTION_ATTR_METER:
+ if (ovs_meter_execute(dp, skb, key, nla_get_u32(a))) {
+ consume_skb(skb);
+ return 0;
+ }
}
if (unlikely(err)) {
@@ -1341,6 +1347,12 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
err = do_execute_actions(dp, skb, key,
acts->actions, acts->actions_len);
+ /* OVS action has dropped the packet, do not expose it
+ * to the user.
+ */
+ if (err == -ENODATA)
+ err = 0;
+
if (level == 1)
process_deferred_actions(dp);
diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index d1ffa1d9fe57..cda40c6af40a 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -30,6 +30,7 @@
#include "conntrack.h"
#include "flow.h"
#include "flow_table.h"
+#include "meter.h"
#include "vport-internal_dev.h"
#define DP_MAX_PORTS USHRT_MAX
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index e8eb427ce6d1..39b548431f68 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -85,6 +85,7 @@ static bool actions_may_change_flow(const struct nlattr *actions)
case OVS_ACTION_ATTR_SAMPLE:
case OVS_ACTION_ATTR_SET:
case OVS_ACTION_ATTR_SET_MASKED:
+ case OVS_ACTION_ATTR_METER:
default:
return true;
}
@@ -2482,6 +2483,7 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
[OVS_ACTION_ATTR_TRUNC] = sizeof(struct ovs_action_trunc),
[OVS_ACTION_ATTR_PUSH_ETH] = sizeof(struct ovs_action_push_eth),
[OVS_ACTION_ATTR_POP_ETH] = 0,
+ [OVS_ACTION_ATTR_METER] = sizeof(u32),
};
const struct ovs_action_push_vlan *vlan;
int type = nla_type(a);
@@ -2636,6 +2638,10 @@ static int __ovs_nla_copy_actions(struct net *net, const struct nlattr *attr,
mac_proto = MAC_PROTO_ETHERNET;
break;
+ case OVS_ACTION_ATTR_METER:
+ /* Non-existent meters are simply ignored. */
+ break;
+
default:
OVS_NLERR(log, "Unknown Action type %d", type);
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: set random switch address
From: Florian Fainelli @ 2017-10-12 22:43 UTC (permalink / raw)
To: Vivien Didelot, netdev; +Cc: linux-kernel, kernel, David S. Miller, Andrew Lunn
In-Reply-To: <87k200vvsl.fsf@weeman.i-did-not-set--mail-host-address--so-tickle-me>
On 10/12/2017 03:35 PM, Vivien Didelot wrote:
> Hi Florian,
>
> Florian Fainelli <f.fainelli@gmail.com> writes:
>
>> On 10/12/2017 03:10 PM, Vivien Didelot wrote:
>>> An Ethernet switch may support having a MAC address, which can be used
>>> as the switch's source address in transmitted full-duplex Pause frames.
>>>
>>> If a DSA switch supports the related .set_addr operation, the DSA core
>>> sets the master's MAC address on the switch.
>>>
>>> This won't make sense anymore in a multi-CPU ports system, because there
>>> won't be a unique master device assigned to a switch tree.
>>
>> Thus far, everything you have said is true, but why we should do it,
>> that is: what if we don't, needs to be explained. Does that create a
>> problem with the generation of pause frames throughout the switch fabric?
>>
>>>
>>> To fix this, assign a random MAC address to the switch chip instead.
>>
>> Maybe this is something that should be removed entirely from the DSA
>> core and pushed into the individual switch drivers instead. dsa_loop
>> implements it for code coverage, but that does not do anything.
>>
>> set_addr is confusing in that you may think it could be used to program
>> the switch with the MAC address of the CPU/management port such that you
>> can disable MAC address learning on said port, but in fact, that's not
>> how it is used.
>
> You are correct. So what I can do is assign a random MAC address in the
> Marvell driver, remove the .set_addr implementation of mv88e6xxx and
> dsa_loop, and finally remove this code from DSA core completely.
Works for me, thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH] Add -target to clang switches while cross compiling.
From: Abhijit Ayarekar @ 2017-10-12 22:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Abhijit Ayarekar, ast, daniel, netdev, linux-kernel, yhs
In-Reply-To: <20171012222302.3zlndmcm7ixywbs6@ast-mbp>
On Thu, Oct 12, 2017 at 03:23:04PM -0700, Alexei Starovoitov wrote:
> On Thu, Oct 12, 2017 at 01:45:57PM -0700, Abhijit Ayarekar wrote:
> > Latest llvm update excludes assembly instructions.
> > As a result __ASM_SYSREGS_H define is not required.
> > -target switch includes appropriate target specific files.
> >
> > Tested on x86 and arm64 with llvm with git revision
> > commit df6ca162269f9d756f8742bf4b658dcf690e3eb5
> > Author: Yonghong Song <yhs@fb.com>
> > Date: Thu Sep 28 02:46:11 2017 +0000
> >
> > bpf: add new insns for bswap_to_le and negation
> >
> > Signed-off-by: Abhijit Ayarekar <abhijit.ayarekar@caviumnetworks.com>
> > ---
> > samples/bpf/Makefile | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
> > index ebc2ad6..81f9fcd 100644
> > --- a/samples/bpf/Makefile
> > +++ b/samples/bpf/Makefile
> > @@ -180,6 +180,7 @@ CLANG ?= clang
> > # Detect that we're cross compiling and use the cross compiler
> > ifdef CROSS_COMPILE
> > HOSTCC = $(CROSS_COMPILE)gcc
> > +CLANG_ARCH_ARGS = -target $(ARCH)
>
> this is only need because you're crosscompiling, right?
Yes
> In native compilation it's unnecessary flag.
> Only droping -D__ASM_SYSREG_H is enough, correct?
>
Yes. That is correct.
> > endif
> >
> > # Trick to allow make to be run from this directory
> > @@ -229,9 +230,9 @@ $(obj)/tracex5_kern.o: $(obj)/syscall_nrs.h
> > $(obj)/%.o: $(src)/%.c
> > $(CLANG) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) $(EXTRA_CFLAGS) -I$(obj) \
> > -I$(srctree)/tools/testing/selftests/bpf/ \
> > - -D__KERNEL__ -D__ASM_SYSREG_H -Wno-unused-value -Wno-pointer-sign \
> > + -D__KERNEL__ -Wno-unused-value -Wno-pointer-sign \
> > -D__TARGET_ARCH_$(ARCH) -Wno-compare-distinct-pointer-types \
> > -Wno-gnu-variable-sized-type-not-at-end \
> > -Wno-address-of-packed-member -Wno-tautological-compare \
> > - -Wno-unknown-warning-option \
> > + -Wno-unknown-warning-option $(CLANG_ARCH_ARGS) \
> > -O2 -emit-llvm -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> > --
> > 2.7.4
> >
^ 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