* Re: [PATCH iproute2 net-next v1 0/7] tipc: updates for neighbour monitor
From: Stephen Hemminger @ 2016-09-20 16:39 UTC (permalink / raw)
To: Parthasarathy Bhuvaragan; +Cc: netdev, tipc-discussion, jon.maloy, maloy
In-Reply-To: <1473693441-14254-1-git-send-email-parthasarathy.bhuvaragan@ericsson.com>
On Mon, 12 Sep 2016 17:17:14 +0200
Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com> wrote:
> We add configuration support for the new link monitoring attributes.
>
> Parthasarathy Bhuvaragan (7):
> tipc: remove dead code
> tipc: add link monitor set threshold
> tipc: add link monitor get threshold
> tipc: add link monitor summary
> tipc: refractor bearer to facilitate link monitor
> tipc: add link monitor list
> tipc: update man page for link monitor
>
> man/man8/tipc-link.8 | 104 +++++++++++++
> tipc/bearer.c | 75 ++++++----
> tipc/bearer.h | 4 +
> tipc/link.c | 408 ++++++++++++++++++++++++++++++++++++++++++++++++++-
> 4 files changed, 556 insertions(+), 35 deletions(-)
>
Applied, then I did a style scrub on tipc/link.c
^ permalink raw reply
* Re: [PATCH bug-fix] iproute: fix documentation for ip rule scan order
From: Stephen Hemminger @ 2016-09-20 16:38 UTC (permalink / raw)
To: Phil Sutter; +Cc: Michal Kubecek, Iskren Chernev, netdev
In-Reply-To: <20160908124317.GK5252@orbyte.nwl.cc>
On Thu, 8 Sep 2016 14:43:17 +0200
Phil Sutter <phil@nwl.cc> wrote:
> On Thu, Sep 08, 2016 at 01:48:08PM +0200, Michal Kubecek wrote:
> > On Thu, Sep 08, 2016 at 12:33:03PM +0200, Phil Sutter wrote:
> > > On Thu, Sep 08, 2016 at 11:59:55AM +0200, Michal Kubecek wrote:
> > > >
> > > > I'm sorry I didn't notice before but this just reverts the change done
> > > > by commit 49572501664d ("iproute2: clarification of various man8 pages").
> > > > IMHO the problem is that both versions are equally confusing as the word
> > > > "priority" can be understood in two different senses.
> > > >
> > > > How about more explicit formulation, e.g.
> > > >
> > > > ... in order of decreasing logical priority (i.e. increasing numeric
> > > > values).
> > > >
> > > > Would that be better?
> > >
> > > Looks like the real issue is missing definition of priority. What about
> > > this:
> > >
> > > diff --git a/man/man8/ip-rule.8 b/man/man8/ip-rule.8
> > > index 3508d8090fd2c..13fe9f7f892ee 100644
> > > --- a/man/man8/ip-rule.8
> > > +++ b/man/man8/ip-rule.8
> > > @@ -93,7 +93,7 @@ Each policy routing rule consists of a
> > > .B selector
> > > and an
> > > .B action predicate.
> > > -The RPDB is scanned in order of increasing priority. The selector
> > > +The RPDB is scanned in order of decreasing priority. The selector
> > > of each rule is applied to {source address, destination address, incoming
> > > interface, tos, fwmark} and, if the selector matches the packet,
> > > the action is performed. The action predicate may return with success.
> > > @@ -221,8 +221,10 @@ value to match.
> > >
> > > .TP
> > > .BI priority " PREFERENCE"
> > > -the priority of this rule. Each rule should have an explicitly
> > > -set
> > > +the priority of this rule.
> > > +.I PREFERENCE
> > > +is an unsigned integer value, higher number means lower priority. Each rule
> > > +should have an explicitly set
> > > .I unique
> > > priority value.
> > > The options preference and order are synonyms with priority.
> >
> > Formally, this would be certainly sufficient. But for clarity (and
> > inattentive readers), I would still prefer to be more explicit in the
> > first hunk, e.g.
> >
> > ... in order of decreasing priority (increasing PREFERENCE values).
>
> I'm fine with that, though fear mentioning PREFERENCE here might confuse
> readers. I'd go with "i.e. increasing numeric values" instead. But after
> all this is quite a discussion for such a tiny bit of documentation. :)
>
> Cheers, Phil
I put in the documentation change, if you want to modify send another patch.
^ permalink raw reply
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Sowmini Varadhan @ 2016-09-20 16:31 UTC (permalink / raw)
To: Jiri Benc; +Cc: netdev, davem, hannes, aduyck, daniel, pabeni
In-Reply-To: <20160920181114.0ac7cfa0@griffin>
On (09/20/16 18:11), Jiri Benc wrote:
> > The vxlan header is at offset (14 + 20 + 8) into the packet,
> > so the vxh is not aligned in vxlan_build_skb. Use [get/put]_unaligned
> > functions to modify flags and vni field in the vxh.
>
> How did you calculate that? IP header should be aligned to 4 bytes, UDP
> header is 8 bytes, thus VXLAN header is also aligned to 4 bytes.
The vxlan header is after the ethernet header (14 bytes),
IP header (20 bytes, assuming no options) and udp header (8 bytes).
Post the skb_reserve adjustments (see computations in in mld_newpack(),
for example), this triggers an unaligned access on sparc.
> If you went this way, it would be better to make two local variables
> for vx_flags and vx_vni, store to them and do a single put_unaligned
> after the condition. That way, you would have two less put_unaligned and
> no get_unaligned in the remote csum case.
Ok, that is certainly possible.
> And the code would be
> cleaner. And you're missing vx_flags being accessed in
> vxlan_build_gbp_hdr.
Sure, and there's also potential unaligned access in vxlan_build_gpe_hdr.
But as I was telling Tom, this problem is much deeper than this fix, and
I dont have the facility, at the moment, to test out every one of these
code paths. We would have to fix these, one at a time, in subsequent
patches. This one just fixes the top-level basic code paths.
> But I think this is not needed at all, see above.
>
> Jiri
^ permalink raw reply
* Re: [RFC v2 06/12] qedr: Add support for QP verbs
From: Leon Romanovsky @ 2016-09-20 16:30 UTC (permalink / raw)
To: Ram Amrani
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA,
Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA,
Yuval.Mintz-YGCgFSpz5w/QT0dZR+AlfA,
rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474367764-9555-7-git-send-email-Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]
On Tue, Sep 20, 2016 at 01:35:58PM +0300, Ram Amrani wrote:
> From: Ram amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>
> Add support for Queue Pair verbs which adds, deletes,
> modifies and queries Queue Pairs.
>
> Signed-off-by: Rajesh Borundia <rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Ram Amrani <Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
> ---
> drivers/infiniband/hw/qedr/main.c | 15 +-
> drivers/infiniband/hw/qedr/qedr.h | 126 +++
> drivers/infiniband/hw/qedr/qedr_cm.h | 40 +
> drivers/infiniband/hw/qedr/qedr_hsi_rdma.h | 11 +
> drivers/infiniband/hw/qedr/verbs.c | 1095 ++++++++++++++++++++++++-
> drivers/infiniband/hw/qedr/verbs.h | 7 +
> drivers/net/ethernet/qlogic/qed/qed_cxt.h | 1 +
> drivers/net/ethernet/qlogic/qed/qed_roce.c | 1211 ++++++++++++++++++++++++++++
> drivers/net/ethernet/qlogic/qed/qed_roce.h | 71 ++
> include/linux/qed/qed_roce_if.h | 144 ++++
> include/uapi/rdma/providers/qedr-abi.h | 35 +
Please be aware that the last version for ABI header files doesn't have
"provider" name in directory path (include/uapi/rdma/qedr-abi.h)
> 11 files changed, 2753 insertions(+), 3 deletions(-)
> create mode 100644 drivers/infiniband/hw/qedr/qedr_cm.h
>
> diff --git a/drivers/infiniband/hw/qedr/main.c b/drivers/infiniband/hw/qedr/main.c
> index c99dd6a..10ad9ed 100644
> --- a/drivers/infiniband/hw/qedr/main.c
> +++ b/drivers/infiniband/hw/qedr/main.c
> @@ -52,7 +52,7 @@ uint debug;
> module_param(debug, uint, 0);
> MODULE_PARM_DESC(debug, "Default debug msglevel");
>
> -static LIST_HEAD(qedr_dev_list);
You are removing code which was added in previous patches, why do you do
it?
> +#define QEDR_WQ_MULTIPLIER_DFT (3)
>
> void qedr_ib_dispatch_event(struct qedr_dev *dev, u8 port_num,
> enum ib_event_type type)
> @@ -99,7 +99,11 @@ static int qedr_register_device(struct qedr_dev *dev)
> QEDR_UVERBS(CREATE_CQ) |
> QEDR_UVERBS(RESIZE_CQ) |
> QEDR_UVERBS(DESTROY_CQ) |
> - QEDR_UVERBS(REQ_NOTIFY_CQ);
> + QEDR_UVERBS(REQ_NOTIFY_CQ) |
> + QEDR_UVERBS(CREATE_QP) |
> + QEDR_UVERBS(MODIFY_QP) |
> + QEDR_UVERBS(QUERY_QP) |
> + QEDR_UVERBS(DESTROY_QP);
>
> dev->ibdev.phys_port_cnt = 1;
> dev->ibdev.num_comp_vectors = dev->num_cnq;
> @@ -124,6 +128,11 @@ static int qedr_register_device(struct qedr_dev *dev)
> dev->ibdev.resize_cq = qedr_resize_cq;
> dev->ibdev.req_notify_cq = qedr_arm_cq;
>
> + dev->ibdev.create_qp = qedr_create_qp;
> + dev->ibdev.modify_qp = qedr_modify_qp;
> + dev->ibdev.query_qp = qedr_query_qp;
> + dev->ibdev.destroy_qp = qedr_destroy_qp;
> +
> dev->ibdev.query_pkey = qedr_query_pkey;
>
> dev->ibdev.dma_device = &dev->pdev->dev;
> @@ -619,6 +628,8 @@ static struct qedr_dev *qedr_add(struct qed_dev *cdev, struct pci_dev *pdev,
> goto init_err;
> }
>
> + dev->wq_multiplier = QEDR_WQ_MULTIPLIER_DFT;
> +
> qedr_pci_set_atomic(dev, pdev);
>
> rc = qedr_alloc_resources(dev);
> diff --git a/drivers/infiniband/hw/qedr/qedr.h b/drivers/infiniband/hw/qedr/qedr.h
> index dd974d5..05017be 100644
> --- a/drivers/infiniband/hw/qedr/qedr.h
> +++ b/drivers/infiniband/hw/qedr/qedr.h
> @@ -49,6 +49,9 @@
> enum DP_QEDR_MODULE {
> QEDR_MSG_INIT = 0x10000,
We prefer shift notation (1 << 16) instead of 0x10000.
> QEDR_MSG_CQ = 0x20000,
> + QEDR_MSG_RQ = 0x40000,
> + QEDR_MSG_SQ = 0x80000,
> + QEDR_MSG_QP = (QEDR_MSG_SQ | QEDR_MSG_RQ),
> QEDR_MSG_MR = 0x100000,
> QEDR_MSG_MISC = 0x400000,
> };
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Jesper Dangaard Brouer @ 2016-09-20 16:27 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Alexei Starovoitov, Tariq Toukan, Tariq Toukan,
David S. Miller, Linux Kernel Network Developers, Eran Ben Elisha,
Saeed Mahameed, Rana Shahout, brouer
In-Reply-To: <1474388036.23058.26.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 20 Sep 2016 09:13:56 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2016-09-20 at 18:06 +0200, Jesper Dangaard Brouer wrote:
> > On Tue, 20 Sep 2016 08:58:30 -0700
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> > > Same for XDP_TX if/when packet is dropped because output ring is full.
> >
> > For the XDP_TX case a counter is already incremented[1] but it is a
> > local ring counter (ring->tx_dropped++).
> >
> > Do you think we should maintain separate counters for XDP? (to have a
> > more consistent interface across drivers...)
>
> No, as long as the admin can learn drops are occurring.
Okay, so recording these drops is important for an admin, agreed. Now
that we have the chance to define the API, wouldn't is be nice if the
admin across drive drivers knew what counter to look for???
> "perf ... -e skb:kfree_skb ..." or drop monitor wont work,
> unfortunately.
That is actually a good idea... why not add a trace point for these
rare drop cases, which is a hassle to debug for an admin?
Let's actually take advantage of all the nice infrastructure the kernel
provides(?)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: UBSAN reports issue in ip_idents_reserve
From: Jiri Pirko @ 2016-09-20 16:25 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1474385116.23058.23.camel@edumazet-glaptop3.roam.corp.google.com>
Tue, Sep 20, 2016 at 05:25:16PM CEST, eric.dumazet@gmail.com wrote:
>On Tue, 2016-09-20 at 07:11 -0700, Eric Dumazet wrote:
>> On Tue, 2016-09-20 at 15:39 +0200, Jiri Pirko wrote:
>>
>> > I see. So how to silent the warning?
>> >
>>
>> We can replace the atomic_add_return() and use a loop around
>> atomic_read() and atomic_cmpxhg()
>>
>> This would change the nice property of x86 xadd into a loop.
>>
>> Or we also could fallback to random generation if the atomic_cmpxchg()
>> fails.
>>
>> I'll provide a patch, thanks.
>>
I'm going to test your patch now.
>
>I looks at other places, I am surprised you do not see other UBSAN
>issues in networking :)
Not yet :)
>
>netdev_refcnt_read() can potentially gives errors as well.
>
>
>
^ permalink raw reply
* Re: [RFC v2 00/11] QLogic RDMA Driver (qedr) RFC
From: Leon Romanovsky @ 2016-09-20 16:23 UTC (permalink / raw)
To: Elior, Ariel
Cc: Amrani, Ram, davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
Kalderon, Michal, Mintz, Yuval, Borundia, Rajesh,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CY1PR0701MB133773B2CB71FA6362CADCFF90F70-UpKza+2NMNLi6bjPjkn3FE5OhdzP3rhOnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]
On Tue, Sep 20, 2016 at 01:33:25PM +0000, Elior, Ariel wrote:
> > On Tue, Sep 20, 2016 at 01:35:52PM +0300, Ram Amrani wrote:
> ...
> > > The series adds on top of RFC v1:
> > > * a check for all drivers that IB_ACCESS_MW_BIND isn't set for ib_get_dma_mr
> > > * relocation of qedr user API to include/rdma/uapi/providers/
> > > * removal of qedr_devlist_local
> > > * fixed error handling in qedr_alloc_resources()
> > > * configuration of PBL in ib_map_mr_sg() driver implementation,
> > > rather than post_send's IB_WR_REG_MR
> > > * misc.: placed code in proper patch, fixed a few comments,
> > > removed extra parentheses
> > >
> > > Thanks for everyone which pointed out problems in the driver.
> > >
> > > Any review/comment is appreciated.
> >
> > Very nice,
> > Any reason why didn't you drop debug module parameter and decided to
> > mimic already available kernel core functionality?
> >
> > You got technical explanations why it is bad idea to use it. If you need additional
> > voices
> > to support my claims, you will find them in thread about VERBOSE flag and responses
> > from
> > Doug, and Dennis.
> >
> > Thanks
> Hi Leon,
> The RFC cover letter lists what has been addressed. Debug printouts are not addressed in V2 as the discussion on that topic is not concluded (more thoughts from us on debug printouts incoming on the thread). There were many comments to V1 which are not relevant to debug printouts which are addressed by V2. We are requesting further comment, hence RFC V2. Rest assured, if it is the final opinion in the relevant discussion that pr_debug is the way to go, that's what we'll do.
The module parameters is no-go.
> Thanks,
> Ariel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Eric Dumazet @ 2016-09-20 16:13 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: Tom Herbert, Alexei Starovoitov, Tariq Toukan, Tariq Toukan,
David S. Miller, Linux Kernel Network Developers, Eran Ben Elisha,
Saeed Mahameed, Rana Shahout
In-Reply-To: <20160920180609.148874b5@redhat.com>
On Tue, 2016-09-20 at 18:06 +0200, Jesper Dangaard Brouer wrote:
> On Tue, 20 Sep 2016 08:58:30 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Same for XDP_TX if/when packet is dropped because output ring is full.
>
> For the XDP_TX case a counter is already incremented[1] but it is a
> local ring counter (ring->tx_dropped++).
>
> Do you think we should maintain separate counters for XDP? (to have a
> more consistent interface across drivers...)
No, as long as the admin can learn drops are occurring.
"perf ... -e skb:kfree_skb ..." or drop monitor wont work,
unfortunately.
^ permalink raw reply
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Jiri Benc @ 2016-09-20 16:11 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: netdev, davem, hannes, aduyck, daniel, pabeni
In-Reply-To: <20160920142700.GJ8920@oracle.com>
On Tue, 20 Sep 2016 10:27:00 -0400, Sowmini Varadhan wrote:
> The vxlan header is at offset (14 + 20 + 8) into the packet,
> so the vxh is not aligned in vxlan_build_skb. Use [get/put]_unaligned
> functions to modify flags and vni field in the vxh.
How did you calculate that? IP header should be aligned to 4 bytes, UDP
header is 8 bytes, thus VXLAN header is also aligned to 4 bytes.
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index e7d1668..f903fa4 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1751,15 +1751,17 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
> goto out_free;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> - vxh->vx_flags = VXLAN_HF_VNI;
> - vxh->vx_vni = vxlan_vni_field(vni);
> + put_unaligned(VXLAN_HF_VNI, &vxh->vx_flags);
> + put_unaligned(vxlan_vni_field(vni), &vxh->vx_vni);
>
> if (type & SKB_GSO_TUNNEL_REMCSUM) {
> unsigned int start;
> + __be32 tmpvni = get_unaligned(&vxh->vx_vni);
>
> start = skb_checksum_start_offset(skb) - sizeof(struct vxlanhdr);
> - vxh->vx_vni |= vxlan_compute_rco(start, skb->csum_offset);
> - vxh->vx_flags |= VXLAN_HF_RCO;
> + tmpvni |= vxlan_compute_rco(start, skb->csum_offset);
> + put_unaligned(tmpvni, &vxh->vx_vni);
> + put_unaligned(VXLAN_HF_VNI | VXLAN_HF_RCO, &vxh->vx_flags);
If you went this way, it would be better to make two local variables
for vx_flags and vx_vni, store to them and do a single put_unaligned
after the condition. That way, you would have two less put_unaligned and
no get_unaligned in the remote csum case. And the code would be
cleaner. And you're missing vx_flags being accessed in
vxlan_build_gbp_hdr.
But I think this is not needed at all, see above.
Jiri
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Jesper Dangaard Brouer @ 2016-09-20 16:06 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, Alexei Starovoitov, Tariq Toukan, Tariq Toukan,
David S. Miller, Linux Kernel Network Developers, Eran Ben Elisha,
Saeed Mahameed, Rana Shahout, brouer
In-Reply-To: <1474387110.23058.24.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 20 Sep 2016 08:58:30 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2016-09-20 at 08:51 -0700, Tom Herbert wrote:
> > On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > > On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
> > >> >>>+ case XDP_ABORTED:
> > >> >>It is not clearly defined, but I believe XDP_ABORTED should also result
> > >> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
> > >> I'll add this.
> > >
> > > Certainly NOT.
> > > XDP_ABORTED is an exception case when program does divide by zero.
> > > It should NOT do bpf_warn. It must drop the packet.
> > > We discussed it several months ago.
> > > See mlx4/en_rx.c for canonical implementation.
> > >
> > It should at least bump a counter so that the user knows that aborts
> > are happening.
I agree.
> Same for XDP_TX if/when packet is dropped because output ring is full.
For the XDP_TX case a counter is already incremented[1] but it is a
local ring counter (ring->tx_dropped++).
Do you think we should maintain separate counters for XDP? (to have a
more consistent interface across drivers...)
[1] https://github.com/torvalds/linux/blob/master/drivers/net/ethernet/mellanox/mlx4/en_tx.c#L1181
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Alexei Starovoitov @ 2016-09-20 16:00 UTC (permalink / raw)
To: Tom Herbert
Cc: Tariq Toukan, Jesper Dangaard Brouer, Tariq Toukan,
David S. Miller, Linux Kernel Network Developers, Eran Ben Elisha,
Saeed Mahameed, Rana Shahout
In-Reply-To: <CALx6S34YBsmSJr7z2wGsu1VvLkjoqTN+Q8JJ0j-H_DehMJzQ5g@mail.gmail.com>
On Tue, Sep 20, 2016 at 08:51:05AM -0700, Tom Herbert wrote:
> On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
> >> >>>+ case XDP_ABORTED:
> >> >>It is not clearly defined, but I believe XDP_ABORTED should also result
> >> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
> >> I'll add this.
> >
> > Certainly NOT.
> > XDP_ABORTED is an exception case when program does divide by zero.
> > It should NOT do bpf_warn. It must drop the packet.
> > We discussed it several months ago.
> > See mlx4/en_rx.c for canonical implementation.
> >
> It should at least bump a counter so that the user knows that aborts
> are happening.
yes the driver can add another counter, but it's not mandated by xdp side.
Meaning it's up to the driver to have counters and their way of
reporting thems (so far all drivers do it via ethtool).
We don't define any counters via XDP api, since they cannot be defined
in a common way across different nics and hw, and we already have ethtool
which is good enough. Especially in this case 'aborted' counter is
only for debugging. The program should not have 'divide by zero' when
it's written correctly.
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Eric Dumazet @ 2016-09-20 15:58 UTC (permalink / raw)
To: Tom Herbert
Cc: Alexei Starovoitov, Tariq Toukan, Jesper Dangaard Brouer,
Tariq Toukan, David S. Miller, Linux Kernel Network Developers,
Eran Ben Elisha, Saeed Mahameed, Rana Shahout
In-Reply-To: <CALx6S34YBsmSJr7z2wGsu1VvLkjoqTN+Q8JJ0j-H_DehMJzQ5g@mail.gmail.com>
On Tue, 2016-09-20 at 08:51 -0700, Tom Herbert wrote:
> On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> > On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
> >> >>>+ case XDP_ABORTED:
> >> >>It is not clearly defined, but I believe XDP_ABORTED should also result
> >> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
> >> I'll add this.
> >
> > Certainly NOT.
> > XDP_ABORTED is an exception case when program does divide by zero.
> > It should NOT do bpf_warn. It must drop the packet.
> > We discussed it several months ago.
> > See mlx4/en_rx.c for canonical implementation.
> >
> It should at least bump a counter so that the user knows that aborts
> are happening.
Same for XDP_TX if/when packet is dropped because output ring is full.
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Jesper Dangaard Brouer @ 2016-09-20 15:57 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Tariq Toukan, Tariq Toukan, David S. Miller, netdev,
Eran Ben Elisha, Saeed Mahameed, Rana Shahout, brouer
In-Reply-To: <20160920154036.GA98644@ast-mbp.thefacebook.com>
On Tue, 20 Sep 2016 08:40:37 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
> > >>>+ case XDP_ABORTED:
> > >>It is not clearly defined, but I believe XDP_ABORTED should also result
> > >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
> > I'll add this.
>
> Certainly NOT.
> XDP_ABORTED is an exception case when program does divide by zero.
> It should NOT do bpf_warn. It must drop the packet.
> We discussed it several months ago.
> See mlx4/en_rx.c for canonical implementation.
It was certainly not documented, and my memory fails me.
Please explain why a eBPF program error (div by zero) must be a silent drop?
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* [PATCH net-next] net: ethernet: mediatek: fix missing changes merged for conflicts overlapping commits
From: sean.wang @ 2016-09-20 15:53 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
add the missing commits about
1)
Commit d3bd1ce4db8e843dce421e2f8f123e5251a9c7d3
("remove redundant free_irq for devm_request_ir allocated irq")
2)
Commit 7c6b0d76fa02213393815e3b6d5e4a415bf3f0e2
("fix logic unbalance between probe and remove")
during merge for conflicts overlapping commits by
Commit b20b378d49926b82c0a131492fa8842156e0e8a9
("Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net")
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 609fd2b..b831a8b 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1945,11 +1945,8 @@ static void mtk_uninit(struct net_device *dev)
struct mtk_eth *eth = mac->hw;
phy_disconnect(mac->phy_dev);
- mtk_mdio_cleanup(eth);
mtk_irq_disable(eth, MTK_QDMA_INT_MASK, ~0);
mtk_irq_disable(eth, MTK_PDMA_INT_MASK, ~0);
- free_irq(eth->irq[1], dev);
- free_irq(eth->irq[2], dev);
}
static int mtk_do_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
@@ -2505,6 +2502,7 @@ static int mtk_remove(struct platform_device *pdev)
netif_napi_del(ð->tx_napi);
netif_napi_del(ð->rx_napi);
mtk_cleanup(eth);
+ mtk_mdio_cleanup(eth);
return 0;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Tom Herbert @ 2016-09-20 15:51 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Tariq Toukan, Jesper Dangaard Brouer, Tariq Toukan,
David S. Miller, Linux Kernel Network Developers, Eran Ben Elisha,
Saeed Mahameed, Rana Shahout
In-Reply-To: <20160920154036.GA98644@ast-mbp.thefacebook.com>
On Tue, Sep 20, 2016 at 8:40 AM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
>> >>>+ case XDP_ABORTED:
>> >>It is not clearly defined, but I believe XDP_ABORTED should also result
>> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
>> I'll add this.
>
> Certainly NOT.
> XDP_ABORTED is an exception case when program does divide by zero.
> It should NOT do bpf_warn. It must drop the packet.
> We discussed it several months ago.
> See mlx4/en_rx.c for canonical implementation.
>
It should at least bump a counter so that the user knows that aborts
are happening.
^ permalink raw reply
* [RFC] PCI: Allow sysfs control over totalvfs
From: Yuval Mintz @ 2016-09-20 15:49 UTC (permalink / raw)
To: linux-pci; +Cc: netdev, derek.chickles, Yuval Mintz, Yuval Mintz
[Sorry in advance if this was already discussed in the past]
Some of the HW capable of SRIOV has resource limitations, where the
PF and VFs resources are drawn from a common pool.
In some cases, these limitations have to be considered early during
chip initialization and can only be changed by tearing down the
configuration and re-initializing.
As a result, drivers for such HWs sometimes have to make unfavorable
compromises where they reserve sufficient resources to accomadate
the maximal number of VFs that can be created - at the expanse of
resources that could have been used by the PF.
If users were able to provide 'hints' regarding the required number
of VFs *prior* to driver attachment, then such compromises could be
avoided. As we already have sysfs nodes that can be queried for the
number of totalvfs, it makes sense to let the user reduce the number
of said totalvfs using same infrastrucure.
Then, we can have drivers supporting SRIOV take that value into account
when deciding how much resources to reserve, allowing the PF to benefit
from the difference between the configuration space value and the actual
number needed by user.
Signed-off-by: Yuval Mintz <Yuval.Mintz@caviumnetworks.com>
---
drivers/pci/pci-sysfs.c | 28 +++++++++++++++++++++++++++-
1 file changed, 27 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index bcd10c7..c1546f8 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -449,6 +449,30 @@ static ssize_t sriov_totalvfs_show(struct device *dev,
return sprintf(buf, "%u\n", pci_sriov_get_totalvfs(pdev));
}
+static ssize_t sriov_totalvfs_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ u16 max_vfs;
+ int ret;
+
+ ret = kstrtou16(buf, 0, &max_vfs);
+ if (ret < 0)
+ return ret;
+
+ if (pdev->driver) {
+ dev_info(&pdev->dev,
+ "Can't change totalvfs while driver is attached\n");
+ return -EUSERS;
+ }
+
+ ret = pci_sriov_set_totalvfs(pdev, max_vfs);
+ if (ret)
+ return ret;
+
+ return count;
+}
static ssize_t sriov_numvfs_show(struct device *dev,
struct device_attribute *attr,
@@ -516,7 +540,9 @@ static ssize_t sriov_numvfs_store(struct device *dev,
return count;
}
-static struct device_attribute sriov_totalvfs_attr = __ATTR_RO(sriov_totalvfs);
+static struct device_attribute sriov_totalvfs_attr =
+ __ATTR(sriov_totalvfs, (S_IRUGO|S_IWUSR|S_IWGRP),
+ sriov_totalvfs_show, sriov_totalvfs_store);
static struct device_attribute sriov_numvfs_attr =
__ATTR(sriov_numvfs, (S_IRUGO|S_IWUSR|S_IWGRP),
sriov_numvfs_show, sriov_numvfs_store);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Sowmini Varadhan @ 2016-09-20 15:49 UTC (permalink / raw)
To: Tom Herbert
Cc: Linux Kernel Network Developers, David S. Miller, Jiri Benc,
Hannes Frederic Sowa, Alexander Duyck, Daniel Borkmann,
Paolo Abeni
In-Reply-To: <CALx6S34c=Ftn1PfQ+YRDvbbWUmFW0R1zGC2jgkY0G7Fcx_SM7g@mail.gmail.com>
On (09/20/16 08:31), Tom Herbert wrote:
>
> On Tue, Sep 20, 2016 at 7:27 AM, Sowmini Varadhan
> <sowmini.varadhan@oracle.com> wrote:
> > The vxlan header is at offset (14 + 20 + 8) into the packet,
> > so the vxh is not aligned in vxlan_build_skb. Use [get/put]_unaligned
> > functions to modify flags and vni field in the vxh.
> >
> I'm wondering if VXLAN is just the tip of the iceberg. Wouldn't this
> is also be a problem in GRE and Geneve when they encapsulate Ethernet?
> What about the outer IP header, wouldn't that potentially have
> unaligned accesses also when creating it?
Indeed. All of the above are true, and we have to conquer
each unaligned access, one at a time.
There's also the over-arching problem of arbitrary encapsulations
introducing intervening headers that throw off alignment.
That would be a design discussion to be had with the ietf.
--Sowmini
^ permalink raw reply
* Re: [PATCH net-next 7/8] net/mlx5e: XDP TX forwarding support
From: Alexei Starovoitov @ 2016-09-20 15:40 UTC (permalink / raw)
To: Tariq Toukan
Cc: Jesper Dangaard Brouer, Tariq Toukan, David S. Miller, netdev,
Eran Ben Elisha, Saeed Mahameed, Rana Shahout
In-Reply-To: <96b40925-0e8e-0230-0701-96c11d6921a1@gmail.com>
On Tue, Sep 20, 2016 at 03:53:10PM +0300, Tariq Toukan wrote:
> >>>+ case XDP_ABORTED:
> >>It is not clearly defined, but I believe XDP_ABORTED should also result
> >>in a warning (calling bpf_warn_invalid_xdp_action(act)).
> I'll add this.
Certainly NOT.
XDP_ABORTED is an exception case when program does divide by zero.
It should NOT do bpf_warn. It must drop the packet.
We discussed it several months ago.
See mlx4/en_rx.c for canonical implementation.
^ permalink raw reply
* RE: [RFC 02/11] Add RoCE driver framework
From: Elior, Ariel @ 2016-09-20 15:04 UTC (permalink / raw)
To: Leon Romanovsky, Mintz, Yuval
Cc: dledford@redhat.com, Mark Bloch, Ram Amrani, David Miller,
Ariel Elior, Michal Kalderon, Rajesh Borundia,
linux-rdma@vger.kernel.org, netdev
In-Reply-To: <20160915054235.GK26069@leon.nu>
> From: Leon Romanovsky [mailto:leon@kernel.org]
> On Thu, Sep 15, 2016 at 05:11:03AM +0000, Mintz, Yuval wrote:
> > > As a summary, I didn't see in your responses any real life example where you will
> > > need global debug level for your driver.
> >
> > Not sure what you you're expecting - a list of BZs /private e-mails where
> > user reproductions were needed?
> > You're basically ignoring my claims that such are used, instead wanting
> > "evidence". I'm not going to try and produce any such.
>
> I asked an example and not evidence, where "modprobe your_driver
> debug=1" will be superior to "modprobe your_driver dyndbg==pmf".
>
> https://www.kernel.org/doc/Documentation/dynamic-debug-howto.txt
Hi,
dyndbg vs module param:
Dynamic debug has two very nice features: dynamic activation/configuration and per line/file/module/format activation. The module param has neither, but it does have a few merits which I am not sure dyndbg has:
(1)
It can activate printouts according to *flow*. A lot of thought has been put into associating the right printout in our driver at the right verbosity level with the right "flow tag" (e.g. QEDR_MSG_INIT, QEDR_MSG_QP). The module parameter accepts a bitmask which allows setting any subset of these flows. This means that with the correct values for the parameter I can open only "init" printouts, or only "Memory Region" printouts, even if these cross multiple files / functions and don't share a common format. Presumably, one would claim that we could add the "flow tag" to the format to every printout according to its flow, but that would encumber the printouts, and also doesn't scale well to printouts which belong to multiple flows, where the current approach allows this (QEDR_MSG_SQ | QEDR_MSG_RQ).
(2)
As Yuval pointed out, there are users out there which have no trouble loading a driver with a module parameter, at probe or at kernel boot, but would be at a loss in mounting debugfs and dumping a matchspec script into a node there. As kernel developers, educating users is part of what we do, but it comes with a cost.
(3)
debugfs can be compiled out of the kernel in some codesize sensitive environments, or may not exist in an emergency shell or kdump kernel, whereas the module parameter would always be there.
Simply allowing our module parameter mechanism to be dynamically activated is very straightforward - we planned on adding a debugfs node for that anyway. But I would keep the module parameter for the sake of those less capable users and also for when debugfs is not available as detailed in (3) above.
I certainly see the merits of joining an existing infrastructure instead of implementing our own thing, but I would like to know if there are ways of obtaining the merits I listed for our approach via that infrastructure.
Leon, aside of commenting on the above, can you give an example of a driver which in your opinion does a good job of using dyndbg?
Thanks,
Ariel
^ permalink raw reply
* Re: [PATCH net-next 8/8] net: qualcomm: add QCA7000 UART driver
From: Marcel Holtmann @ 2016-09-20 15:32 UTC (permalink / raw)
To: Stefan Wahren
Cc: David S. Miller, Greg Kroah-Hartman, Jiri Slaby,
Network Development, LKML, Rob Herring
In-Reply-To: <1474376544-20515-9-git-send-email-stefan.wahren@i2se.com>
Hi Stefan,
> This patch adds the Ethernet over UART driver for the
> Qualcomm QCA7000 HomePlug GreenPHY.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
> drivers/net/ethernet/qualcomm/Kconfig | 12 +
> drivers/net/ethernet/qualcomm/Makefile | 2 +
> drivers/net/ethernet/qualcomm/qca_common.h | 6 +
> drivers/net/ethernet/qualcomm/qca_uart.c | 447 +++++++++++++++++++++++++++++
> include/uapi/linux/tty.h | 1 +
> 5 files changed, 468 insertions(+)
> create mode 100644 drivers/net/ethernet/qualcomm/qca_uart.c
>
> diff --git a/drivers/net/ethernet/qualcomm/Kconfig b/drivers/net/ethernet/qualcomm/Kconfig
> index 0d33728..0ede46e 100644
> --- a/drivers/net/ethernet/qualcomm/Kconfig
> +++ b/drivers/net/ethernet/qualcomm/Kconfig
> @@ -30,6 +30,18 @@ config QCA7000_SPI
> To compile this driver as a module, choose M here. The module
> will be called qcaspi.
>
> +config QCA7000_UART
> + tristate "Qualcomm Atheros QCA7000 UART support"
> + select QCA7000
> + depends on TTY
> + ---help---
> + This UART protocol driver supports the Qualcomm Atheros QCA7000.
> + The driver implements the tty line discipline N_QCA7K and supports
> + only one netdevice.
> +
> + To compile this driver as a module, choose M here. The module
> + will be called qcauart.
> +
this seems to be another candidate for having a proper UART or serial bus. Instead of adding one line discipline after another, I think we need to get quickly to having an enumerable bus here.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH net-next] net/vxlan: Avoid unaligned access in vxlan_build_skb()
From: Tom Herbert @ 2016-09-20 15:31 UTC (permalink / raw)
To: Sowmini Varadhan
Cc: Linux Kernel Network Developers, David S. Miller, Jiri Benc,
Hannes Frederic Sowa, Alexander Duyck, Daniel Borkmann,
Paolo Abeni
In-Reply-To: <20160920142700.GJ8920@oracle.com>
On Tue, Sep 20, 2016 at 7:27 AM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> The vxlan header is at offset (14 + 20 + 8) into the packet,
> so the vxh is not aligned in vxlan_build_skb. Use [get/put]_unaligned
> functions to modify flags and vni field in the vxh.
>
I'm wondering if VXLAN is just the tip of the iceberg. Wouldn't this
is also be a problem in GRE and Geneve when they encapsulate Ethernet?
What about the outer IP header, wouldn't that potentially have
unaligned accesses also when creating it?
Tom
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> drivers/net/vxlan.c | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index e7d1668..f903fa4 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -1751,15 +1751,17 @@ static int vxlan_build_skb(struct sk_buff *skb, struct dst_entry *dst,
> goto out_free;
>
> vxh = (struct vxlanhdr *) __skb_push(skb, sizeof(*vxh));
> - vxh->vx_flags = VXLAN_HF_VNI;
> - vxh->vx_vni = vxlan_vni_field(vni);
> + put_unaligned(VXLAN_HF_VNI, &vxh->vx_flags);
> + put_unaligned(vxlan_vni_field(vni), &vxh->vx_vni);
>
> if (type & SKB_GSO_TUNNEL_REMCSUM) {
> unsigned int start;
> + __be32 tmpvni = get_unaligned(&vxh->vx_vni);
>
> start = skb_checksum_start_offset(skb) - sizeof(struct vxlanhdr);
> - vxh->vx_vni |= vxlan_compute_rco(start, skb->csum_offset);
> - vxh->vx_flags |= VXLAN_HF_RCO;
> + tmpvni |= vxlan_compute_rco(start, skb->csum_offset);
> + put_unaligned(tmpvni, &vxh->vx_vni);
> + put_unaligned(VXLAN_HF_VNI | VXLAN_HF_RCO, &vxh->vx_flags);
>
> if (!skb_is_gso(skb)) {
> skb->ip_summed = CHECKSUM_NONE;
> --
> 1.7.1
>
^ permalink raw reply
* Re: [PATCH net-next 6/7] net/faraday: Fix phy link irq on Aspeed G5 SoCs
From: Andrew Lunn @ 2016-09-20 15:29 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Joel Stanley, davem, gwshan, andrew, netdev, linux-kernel
In-Reply-To: <1474373594.2857.62.camel@kernel.crashing.org>
On Tue, Sep 20, 2016 at 10:13:14PM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2016-09-20 at 16:00 +0930, Joel Stanley wrote:
> > On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
> > continual PHYSTS interrupts:
> >
> > [ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
> > [ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
> > [ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
> > [ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
> >
> > This is because the driver was enabling low-level sensitive interrupt
> > generation where the systems are wired for high-level. All CPU cycles
> > are spent servicing this interrupt.
>
> If this is a system wiring issue, should it be represented by a DT
> property ?
Is there a device tree binding document somewhere?
Is it possible just to put ACTIVE_HIGH in the right place in the
binding?
Andrew
^ permalink raw reply
* Re: [RFC v2 06/12] qedr: Add support for QP verbs
From: Jason Gunthorpe @ 2016-09-20 15:28 UTC (permalink / raw)
To: Ram Amrani
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
Ariel.Elior-YGCgFSpz5w/QT0dZR+AlfA,
Michal.Kalderon-YGCgFSpz5w/QT0dZR+AlfA,
Yuval.Mintz-YGCgFSpz5w/QT0dZR+AlfA,
rajesh.borundia-YGCgFSpz5w/QT0dZR+AlfA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1474367764-9555-7-git-send-email-Ram.Amrani-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
On Tue, Sep 20, 2016 at 01:35:58PM +0300, Ram Amrani wrote:
> +++ b/include/uapi/rdma/providers/qedr-abi.h
> @@ -43,4 +43,39 @@ struct qedr_create_cq_uresp {
> u16 icid;
> };
Ugh, each patch keeps adding to this?
> +struct qedr_create_qp_ureq {
> + u32 qp_handle_hi;
> + u32 qp_handle_lo;
> +
> + /* SQ */
> + /* user space virtual address of SQ buffer */
> + u64 sq_addr;
> +
> + /* length of SQ buffer */
> + size_t sq_len;
Do not use size_t, that is just asking for 32/64 bit interop problems.
> +struct qedr_create_qp_uresp {
> + u32 qp_id;
> + int atomic_supported;
int again, etc. You get the idea.
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC v2 04/12] qedr: Add support for user context verbs
From: Jason Gunthorpe @ 2016-09-20 15:27 UTC (permalink / raw)
To: Ram Amrani
Cc: davem, dledford, Ariel.Elior, Michal.Kalderon, Yuval.Mintz,
rajesh.borundia, linux-rdma, netdev
In-Reply-To: <1474367764-9555-5-git-send-email-Ram.Amrani@cavium.com>
On Tue, Sep 20, 2016 at 01:35:56PM +0300, Ram Amrani wrote:
> +++ b/include/uapi/rdma/providers/qedr-abi.h
> @@ -0,0 +1,27 @@
> +/* QLogic qed NIC Driver
> + * Copyright (c) 2015 QLogic Corporation
> + *
> + * This software is available under the terms of the GNU General Public License
> + * (GPL) Version 2, available from the file COPYING in the main directory of
> + * this source tree.
> + */
> +#ifndef __QEDR_USER_H__
> +#define __QEDR_USER_H__
> +
> +#define QEDR_ABI_VERSION (6)
> +
> +/* user kernel communication data structures. */
> +
> +struct qedr_alloc_ucontext_resp {
> + u64 db_pa;
> + u32 db_size;
> +
> + u32 max_send_wr;
> + u32 max_recv_wr;
> + u32 max_srq_wr;
> + u32 sges_per_send_wr;
> + u32 sges_per_recv_wr;
> + u32 sges_per_srq_wr;
uapi headers need the __ varients and the #include <linux/types.h>
Follow what Leon has done.
> + int max_cqes;
Do not use int here.
Is there really only 1 struct? Your driver never uses udata for
anything else?
Jason
^ permalink raw reply
* Re: UBSAN reports issue in ip_idents_reserve
From: Eric Dumazet @ 2016-09-20 15:25 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev
In-Reply-To: <1474380711.23058.8.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2016-09-20 at 07:11 -0700, Eric Dumazet wrote:
> On Tue, 2016-09-20 at 15:39 +0200, Jiri Pirko wrote:
>
> > I see. So how to silent the warning?
> >
>
> We can replace the atomic_add_return() and use a loop around
> atomic_read() and atomic_cmpxhg()
>
> This would change the nice property of x86 xadd into a loop.
>
> Or we also could fallback to random generation if the atomic_cmpxchg()
> fails.
>
> I'll provide a patch, thanks.
>
I looks at other places, I am surprised you do not see other UBSAN
issues in networking :)
netdev_refcnt_read() can potentially gives errors as well.
^ 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