* RE: [PATCH rdma-next 00/10] Hardware tag matching support
From: Hefty, Sean @ 2016-10-13 17:02 UTC (permalink / raw)
To: Doug Ledford, Leon Romanovsky
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <6259953b-27fe-77c9-ea90-af744f188671-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> I doubt they have, but they change the kernel to user library API. I'm
> not so worried about that since it's entirely under our control. It's
> the user library to application API that I would be more concerned
> with.
I thought there was an agreement that since verbs implies a hardware implementation, then all kernel to user ABI changes to verbs would go through some external organization, such as the OFVWG, for discussion and approval. Has that changed?
- Sean
--
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: Introduction of libqedr to the Consolidated Userspace RDMA Library Repo
From: Jason Gunthorpe @ 2016-10-13 16:29 UTC (permalink / raw)
To: Amrani, Ram
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Borundia, Rajesh,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <SN1PR07MB22074FE9B280DDDCE439FBA9F8DC0-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
On Thu, Oct 13, 2016 at 02:37:24PM +0000, Amrani, Ram wrote:
> > >You will want to look at the various patches I've prepared and ensure
> > >you cover off the basic cleanups that have already been done, and that
> > >your code compiles warning-free on FC24.
>
> By cleanups, do you mean to make sure the code compiles warning-free under
> the various flags mentioned throughout the git log (-Wempty-body and etc.),
Yes, Travis will enforce this.
> or are there other kinds of cleanups that I'm missing?
Yes, many of the drivers copied the same stuff that is now gone. Here
is a sampling
https://github.com/linux-rdma/rdma-core/commit/1df0888f6a736e1612ce8b054d6c17651ebd003f
https://github.com/linux-rdma/rdma-core/commit/6771a2051ea5efd30e142866f722d2dae6f565a7
https://github.com/linux-rdma/rdma-core/commit/f29b3285f82815da3abdfea5be5c4f2d1ca92743
https://github.com/linux-rdma/rdma-core/commit/9358a8a5484d1caa0c7ad1826e07d2105f58cc4e
https://github.com/linux-rdma/rdma-core/commit/c5c7e32796b19c3707620a3d2f6b32d00d2fbc3c
> > >Once you feel everything is ready then post it to the mailing list and
> > >send a pull request. For the mailing list you can just split the
> > >patches by file..
>
> As we don't have a publically accessed server I cannot do this at
> the moment (I'm checking how we can set one up). Is it OK if I send
> you the update as a series of patches?
You don't need a server, just a personal github account.
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
* [PATCH v3] IB/ipoib: move back IB LL address into the hard header
From: Paolo Abeni @ 2016-10-13 16:26 UTC (permalink / raw)
To: linux-rdma
Cc: Doug Ledford, Sean Hefty, Hal Rosenstock, Jason Gunthorpe, netdev,
David Miller
After the commit 9207f9d45b0a ("net: preserve IP control block
during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
That destroy the IPoIB address information cached there,
causing a severe performance regression, as better described here:
http://marc.info/?l=linux-kernel&m=146787279825501&w=2
This change moves the data cached by the IPoIB driver from the
skb control lock into the IPoIB hard header, as done before
the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
and use skb->cb to stash LL addresses").
In order to avoid GRO issue, on packet reception, the IPoIB driver
stash into the skb a dummy pseudo header, so that the received
packets have actually a hard header matching the declared length.
To avoid changing the connected mode maximum mtu, the allocated
head buffer size is increased by the pseudo header length.
After this commit, IPoIB performances are back to pre-regression
value.
v2 -> v3: rebased
v1 -> v2: avoid changing the max mtu, increasing the head buf size
Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
drivers/infiniband/ulp/ipoib/ipoib.h | 20 +++++++---
drivers/infiniband/ulp/ipoib/ipoib_cm.c | 15 +++----
drivers/infiniband/ulp/ipoib/ipoib_ib.c | 12 +++---
drivers/infiniband/ulp/ipoib/ipoib_main.c | 54 ++++++++++++++++----------
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 6 ++-
5 files changed, 64 insertions(+), 43 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h b/drivers/infiniband/ulp/ipoib/ipoib.h
index 7b8d2d9..da12717 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib.h
+++ b/drivers/infiniband/ulp/ipoib/ipoib.h
@@ -63,6 +63,8 @@ enum ipoib_flush_level {
enum {
IPOIB_ENCAP_LEN = 4,
+ IPOIB_PSEUDO_LEN = 20,
+ IPOIB_HARD_LEN = IPOIB_ENCAP_LEN + IPOIB_PSEUDO_LEN,
IPOIB_UD_HEAD_SIZE = IB_GRH_BYTES + IPOIB_ENCAP_LEN,
IPOIB_UD_RX_SG = 2, /* max buffer needed for 4K mtu */
@@ -134,15 +136,21 @@ struct ipoib_header {
u16 reserved;
};
-struct ipoib_cb {
- struct qdisc_skb_cb qdisc_cb;
- u8 hwaddr[INFINIBAND_ALEN];
+struct ipoib_pseudo_header {
+ u8 hwaddr[INFINIBAND_ALEN];
};
-static inline struct ipoib_cb *ipoib_skb_cb(const struct sk_buff *skb)
+static inline void skb_add_pseudo_hdr(struct sk_buff *skb)
{
- BUILD_BUG_ON(sizeof(skb->cb) < sizeof(struct ipoib_cb));
- return (struct ipoib_cb *)skb->cb;
+ char *data = skb_push(skb, IPOIB_PSEUDO_LEN);
+
+ /*
+ * only the ipoib header is present now, make room for a dummy
+ * pseudo header and set skb field accordingly
+ */
+ memset(data, 0, IPOIB_PSEUDO_LEN);
+ skb_reset_mac_header(skb);
+ skb_pull(skb, IPOIB_HARD_LEN);
}
/* Used for all multicast joins (broadcast, IPv4 mcast and IPv6 mcast) */
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_cm.c b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
index 4ad297d..339a1ee 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_cm.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_cm.c
@@ -63,6 +63,8 @@
#define IPOIB_CM_RX_DELAY (3 * 256 * HZ)
#define IPOIB_CM_RX_UPDATE_MASK (0x3)
+#define IPOIB_CM_RX_RESERVE (ALIGN(IPOIB_HARD_LEN, 16) - IPOIB_ENCAP_LEN)
+
static struct ib_qp_attr ipoib_cm_err_attr = {
.qp_state = IB_QPS_ERR
};
@@ -146,15 +148,15 @@ static struct sk_buff *ipoib_cm_alloc_rx_skb(struct net_device *dev,
struct sk_buff *skb;
int i;
- skb = dev_alloc_skb(IPOIB_CM_HEAD_SIZE + 12);
+ skb = dev_alloc_skb(ALIGN(IPOIB_CM_HEAD_SIZE + IPOIB_PSEUDO_LEN, 16));
if (unlikely(!skb))
return NULL;
/*
- * IPoIB adds a 4 byte header. So we need 12 more bytes to align the
+ * IPoIB adds a IPOIB_ENCAP_LEN byte header, this will align the
* IP header to a multiple of 16.
*/
- skb_reserve(skb, 12);
+ skb_reserve(skb, IPOIB_CM_RX_RESERVE);
mapping[0] = ib_dma_map_single(priv->ca, skb->data, IPOIB_CM_HEAD_SIZE,
DMA_FROM_DEVICE);
@@ -624,9 +626,9 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
if (wc->byte_len < IPOIB_CM_COPYBREAK) {
int dlen = wc->byte_len;
- small_skb = dev_alloc_skb(dlen + 12);
+ small_skb = dev_alloc_skb(dlen + IPOIB_CM_RX_RESERVE);
if (small_skb) {
- skb_reserve(small_skb, 12);
+ skb_reserve(small_skb, IPOIB_CM_RX_RESERVE);
ib_dma_sync_single_for_cpu(priv->ca, rx_ring[wr_id].mapping[0],
dlen, DMA_FROM_DEVICE);
skb_copy_from_linear_data(skb, small_skb->data, dlen);
@@ -663,8 +665,7 @@ void ipoib_cm_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
copied:
skb->protocol = ((struct ipoib_header *) skb->data)->proto;
- skb_reset_mac_header(skb);
- skb_pull(skb, IPOIB_ENCAP_LEN);
+ skb_add_pseudo_hdr(skb);
++dev->stats.rx_packets;
dev->stats.rx_bytes += skb->len;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index be11d5d..830fecb 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -128,16 +128,15 @@ static struct sk_buff *ipoib_alloc_rx_skb(struct net_device *dev, int id)
buf_size = IPOIB_UD_BUF_SIZE(priv->max_ib_mtu);
- skb = dev_alloc_skb(buf_size + IPOIB_ENCAP_LEN);
+ skb = dev_alloc_skb(buf_size + IPOIB_HARD_LEN);
if (unlikely(!skb))
return NULL;
/*
- * IB will leave a 40 byte gap for a GRH and IPoIB adds a 4 byte
- * header. So we need 4 more bytes to get to 48 and align the
- * IP header to a multiple of 16.
+ * the IP header will be at IPOIP_HARD_LEN + IB_GRH_BYTES, that is
+ * 64 bytes aligned
*/
- skb_reserve(skb, 4);
+ skb_reserve(skb, sizeof(struct ipoib_pseudo_header));
mapping = priv->rx_ring[id].mapping;
mapping[0] = ib_dma_map_single(priv->ca, skb->data, buf_size,
@@ -253,8 +252,7 @@ static void ipoib_ib_handle_rx_wc(struct net_device *dev, struct ib_wc *wc)
skb_pull(skb, IB_GRH_BYTES);
skb->protocol = ((struct ipoib_header *) skb->data)->proto;
- skb_reset_mac_header(skb);
- skb_pull(skb, IPOIB_ENCAP_LEN);
+ skb_add_pseudo_hdr(skb);
++dev->stats.rx_packets;
dev->stats.rx_bytes += skb->len;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c b/drivers/infiniband/ulp/ipoib/ipoib_main.c
index 5636fc3..b58d9dc 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
@@ -925,9 +925,12 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
ipoib_neigh_free(neigh);
goto err_drop;
}
- if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE)
+ if (skb_queue_len(&neigh->queue) <
+ IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&neigh->queue, skb);
- else {
+ } else {
ipoib_warn(priv, "queue length limit %d. Packet drop.\n",
skb_queue_len(&neigh->queue));
goto err_drop;
@@ -964,7 +967,7 @@ static void neigh_add_path(struct sk_buff *skb, u8 *daddr,
}
static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
- struct ipoib_cb *cb)
+ struct ipoib_pseudo_header *phdr)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_path *path;
@@ -972,16 +975,18 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
spin_lock_irqsave(&priv->lock, flags);
- path = __path_find(dev, cb->hwaddr + 4);
+ path = __path_find(dev, phdr->hwaddr + 4);
if (!path || !path->valid) {
int new_path = 0;
if (!path) {
- path = path_rec_create(dev, cb->hwaddr + 4);
+ path = path_rec_create(dev, phdr->hwaddr + 4);
new_path = 1;
}
if (path) {
if (skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&path->queue, skb);
} else {
++dev->stats.tx_dropped;
@@ -1009,10 +1014,12 @@ static void unicast_arp_send(struct sk_buff *skb, struct net_device *dev,
be16_to_cpu(path->pathrec.dlid));
spin_unlock_irqrestore(&priv->lock, flags);
- ipoib_send(dev, skb, path->ah, IPOIB_QPN(cb->hwaddr));
+ ipoib_send(dev, skb, path->ah, IPOIB_QPN(phdr->hwaddr));
return;
} else if ((path->query || !path_rec_start(dev, path)) &&
skb_queue_len(&path->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, IPOIB_PSEUDO_LEN);
__skb_queue_tail(&path->queue, skb);
} else {
++dev->stats.tx_dropped;
@@ -1026,13 +1033,15 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct ipoib_dev_priv *priv = netdev_priv(dev);
struct ipoib_neigh *neigh;
- struct ipoib_cb *cb = ipoib_skb_cb(skb);
+ struct ipoib_pseudo_header *phdr;
struct ipoib_header *header;
unsigned long flags;
+ phdr = (struct ipoib_pseudo_header *) skb->data;
+ skb_pull(skb, sizeof(*phdr));
header = (struct ipoib_header *) skb->data;
- if (unlikely(cb->hwaddr[4] == 0xff)) {
+ if (unlikely(phdr->hwaddr[4] == 0xff)) {
/* multicast, arrange "if" according to probability */
if ((header->proto != htons(ETH_P_IP)) &&
(header->proto != htons(ETH_P_IPV6)) &&
@@ -1045,13 +1054,13 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
/* Add in the P_Key for multicast*/
- cb->hwaddr[8] = (priv->pkey >> 8) & 0xff;
- cb->hwaddr[9] = priv->pkey & 0xff;
+ phdr->hwaddr[8] = (priv->pkey >> 8) & 0xff;
+ phdr->hwaddr[9] = priv->pkey & 0xff;
- neigh = ipoib_neigh_get(dev, cb->hwaddr);
+ neigh = ipoib_neigh_get(dev, phdr->hwaddr);
if (likely(neigh))
goto send_using_neigh;
- ipoib_mcast_send(dev, cb->hwaddr, skb);
+ ipoib_mcast_send(dev, phdr->hwaddr, skb);
return NETDEV_TX_OK;
}
@@ -1060,16 +1069,16 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
case htons(ETH_P_IP):
case htons(ETH_P_IPV6):
case htons(ETH_P_TIPC):
- neigh = ipoib_neigh_get(dev, cb->hwaddr);
+ neigh = ipoib_neigh_get(dev, phdr->hwaddr);
if (unlikely(!neigh)) {
- neigh_add_path(skb, cb->hwaddr, dev);
+ neigh_add_path(skb, phdr->hwaddr, dev);
return NETDEV_TX_OK;
}
break;
case htons(ETH_P_ARP):
case htons(ETH_P_RARP):
/* for unicast ARP and RARP should always perform path find */
- unicast_arp_send(skb, dev, cb);
+ unicast_arp_send(skb, dev, phdr);
return NETDEV_TX_OK;
default:
/* ethertype not supported by IPoIB */
@@ -1086,11 +1095,13 @@ static int ipoib_start_xmit(struct sk_buff *skb, struct net_device *dev)
goto unref;
}
} else if (neigh->ah) {
- ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(cb->hwaddr));
+ ipoib_send(dev, skb, neigh->ah, IPOIB_QPN(phdr->hwaddr));
goto unref;
}
if (skb_queue_len(&neigh->queue) < IPOIB_MAX_PATH_REC_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, sizeof(*phdr));
spin_lock_irqsave(&priv->lock, flags);
__skb_queue_tail(&neigh->queue, skb);
spin_unlock_irqrestore(&priv->lock, flags);
@@ -1122,8 +1133,8 @@ static int ipoib_hard_header(struct sk_buff *skb,
unsigned short type,
const void *daddr, const void *saddr, unsigned len)
{
+ struct ipoib_pseudo_header *phdr;
struct ipoib_header *header;
- struct ipoib_cb *cb = ipoib_skb_cb(skb);
header = (struct ipoib_header *) skb_push(skb, sizeof *header);
@@ -1132,12 +1143,13 @@ static int ipoib_hard_header(struct sk_buff *skb,
/*
* we don't rely on dst_entry structure, always stuff the
- * destination address into skb->cb so we can figure out where
+ * destination address into skb hard header so we can figure out where
* to send the packet later.
*/
- memcpy(cb->hwaddr, daddr, INFINIBAND_ALEN);
+ phdr = (struct ipoib_pseudo_header *) skb_push(skb, sizeof(*phdr));
+ memcpy(phdr->hwaddr, daddr, INFINIBAND_ALEN);
- return sizeof *header;
+ return IPOIB_HARD_LEN;
}
static void ipoib_set_mcast_list(struct net_device *dev)
@@ -1759,7 +1771,7 @@ void ipoib_setup(struct net_device *dev)
dev->flags |= IFF_BROADCAST | IFF_MULTICAST;
- dev->hard_header_len = IPOIB_ENCAP_LEN;
+ dev->hard_header_len = IPOIB_HARD_LEN;
dev->addr_len = INFINIBAND_ALEN;
dev->type = ARPHRD_INFINIBAND;
dev->tx_queue_len = ipoib_sendq_size * 2;
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
index d3394b6..1909dd2 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
@@ -796,9 +796,11 @@ void ipoib_mcast_send(struct net_device *dev, u8 *daddr, struct sk_buff *skb)
__ipoib_mcast_add(dev, mcast);
list_add_tail(&mcast->list, &priv->multicast_list);
}
- if (skb_queue_len(&mcast->pkt_queue) < IPOIB_MAX_MCAST_QUEUE)
+ if (skb_queue_len(&mcast->pkt_queue) < IPOIB_MAX_MCAST_QUEUE) {
+ /* put pseudoheader back on for next time */
+ skb_push(skb, sizeof(struct ipoib_pseudo_header));
skb_queue_tail(&mcast->pkt_queue, skb);
- else {
+ } else {
++dev->stats.tx_dropped;
dev_kfree_skb_any(skb);
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: David Miller @ 2016-10-13 15:57 UTC (permalink / raw)
To: dledford; +Cc: pabeni, linux-rdma, sean.hefty, hal.rosenstock, netdev
In-Reply-To: <2eadc083-e2e0-5c6d-fe84-c5851be3e2ec@redhat.com>
From: Doug Ledford <dledford@redhat.com>
Date: Thu, 13 Oct 2016 11:20:59 -0400
> We *had* a safe way to do that. It got broken. What about increasing
> the size of skb->cb? Or adding a skb->dgid that is a
> u8[INFINIBAND_ALEN]? Or a more generic skb->dest_ll_addr that is sized
> to hold the dest address for any link layer?
I understand the situation, and I also believe that making sk_buff any
huger than it already is happens to be a non-starter.
>> Pushing metadata before the head of the SKB data pointer is illegal,
>> as the layers in between might want to push protocol headers,
>
> That's a total non-issue for us. There are no headers that protocols
> can add before ours.
Ok, if that's the case, and based upon Paolo's response to me it appears
to be, I guess this is OK for now.
Paolo please resubmit your patch, thanks.
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-13 15:20 UTC (permalink / raw)
To: David Miller
Cc: pabeni-H+wXaHxf7aLQT0dZR+AlfA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161013.104314.1842951254979604965.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 2951 bytes --]
On 10/13/2016 10:43 AM, David Miller wrote:
> From: Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Date: Thu, 13 Oct 2016 10:35:35 -0400
>
>> On 10/13/2016 10:24 AM, David Miller wrote:
>>> From: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>> Date: Tue, 11 Oct 2016 19:15:44 +0200
>>>
>>>> After the commit 9207f9d45b0a ("net: preserve IP control block
>>>> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
>>>> That destroy the IPoIB address information cached there,
>>>> causing a severe performance regression, as better described here:
>>>>
>>>> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
>>>>
>>>> This change moves the data cached by the IPoIB driver from the
>>>> skb control lock into the IPoIB hard header, as done before
>>>> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
>>>> and use skb->cb to stash LL addresses").
>>>> In order to avoid GRO issue, on packet reception, the IPoIB driver
>>>> stash into the skb a dummy pseudo header, so that the received
>>>> packets have actually a hard header matching the declared length.
>>>> Also the connected mode maximum mtu is reduced by 16 bytes to
>>>> cope with the increased hard header len.
>>>>
>>>> After this commit, IPoIB performances are back to pre-regression
>>>> value.
>>>>
>>>> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
>>>> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>>>
>>> Not providing an accurate hard_header_len causes many problems.
>>>
>>> In fact we recently fixed the mlxsw driver to stop doing this.
>>>
>>
>> Sure, but there are too many users of the cb struct, and whatever
>> problems you are saying there are by lying about the hard header len are
>> dwarfed by the problems caused by the inability to store the ll address
>> anywhere between hard_header and send time.
>
> IB wants to pass addressing information between layers, it needs to
> find a safe way to do that.
We *had* a safe way to do that. It got broken. What about increasing
the size of skb->cb? Or adding a skb->dgid that is a
u8[INFINIBAND_ALEN]? Or a more generic skb->dest_ll_addr that is sized
to hold the dest address for any link layer?
> Pushing metadata before the head of the SKB data pointer is illegal,
> as the layers in between might want to push protocol headers,
That's a total non-issue for us. There are no headers that protocols
can add before ours.
> mirror
> the packet to another interface
Doesn't that then mean that the headers specific to this interface
should be stripped before the mirror? If so, I believe the way Paolo
did this patch, that is what will be done.
>, etc.
>
> So this "metadata in SKB data" approach is buggy too.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Paolo Abeni @ 2016-10-13 15:17 UTC (permalink / raw)
To: David Miller; +Cc: linux-rdma, dledford, sean.hefty, hal.rosenstock, netdev
In-Reply-To: <20161013.102432.1450901395774429343.davem@davemloft.net>
On Thu, 2016-10-13 at 10:24 -0400, David Miller wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> Date: Tue, 11 Oct 2016 19:15:44 +0200
>
> > After the commit 9207f9d45b0a ("net: preserve IP control block
> > during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
> > That destroy the IPoIB address information cached there,
> > causing a severe performance regression, as better described here:
> >
> > http://marc.info/?l=linux-kernel&m=146787279825501&w=2
> >
> > This change moves the data cached by the IPoIB driver from the
> > skb control lock into the IPoIB hard header, as done before
> > the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
> > and use skb->cb to stash LL addresses").
> > In order to avoid GRO issue, on packet reception, the IPoIB driver
> > stash into the skb a dummy pseudo header, so that the received
> > packets have actually a hard header matching the declared length.
> > Also the connected mode maximum mtu is reduced by 16 bytes to
> > cope with the increased hard header len.
> >
> > After this commit, IPoIB performances are back to pre-regression
> > value.
> >
> > Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
> > Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>
> Not providing an accurate hard_header_len causes many problems.
>
> In fact we recently fixed the mlxsw driver to stop doing this.
AFAICS the mlxsw did a different thing, adding an additional header in
the xmit function and pushing packets in the network stack with a mac
length different from the declared hard header length
The hard header specified by the IPoIB driver is build in the create()
header_ops and its length matches the mac header length of the packets
pushed into the network stack by such driver. GRO works correctly on top
of that. From the networking stack PoV the hard_header_len should be
'accurate'.
Can you please give more details on the problem that may arise from
this ?
thank you,
Paolo
^ permalink raw reply
* Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header,Re: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: David Miller @ 2016-10-13 14:43 UTC (permalink / raw)
To: dledford; +Cc: pabeni, linux-rdma, sean.hefty, hal.rosenstock, netdev
In-Reply-To: <20ace6ee-b9e4-073c-56e7-29b2c50ae2d5@redhat.com>
From: Doug Ledford <dledford@redhat.com>
Date: Thu, 13 Oct 2016 10:35:35 -0400
> On 10/13/2016 10:24 AM, David Miller wrote:
>> From: Paolo Abeni <pabeni@redhat.com>
>> Date: Tue, 11 Oct 2016 19:15:44 +0200
>>
>>> After the commit 9207f9d45b0a ("net: preserve IP control block
>>> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
>>> That destroy the IPoIB address information cached there,
>>> causing a severe performance regression, as better described here:
>>>
>>> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
>>>
>>> This change moves the data cached by the IPoIB driver from the
>>> skb control lock into the IPoIB hard header, as done before
>>> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
>>> and use skb->cb to stash LL addresses").
>>> In order to avoid GRO issue, on packet reception, the IPoIB driver
>>> stash into the skb a dummy pseudo header, so that the received
>>> packets have actually a hard header matching the declared length.
>>> Also the connected mode maximum mtu is reduced by 16 bytes to
>>> cope with the increased hard header len.
>>>
>>> After this commit, IPoIB performances are back to pre-regression
>>> value.
>>>
>>> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
>>> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
>>
>> Not providing an accurate hard_header_len causes many problems.
>>
>> In fact we recently fixed the mlxsw driver to stop doing this.
>>
>
> Sure, but there are too many users of the cb struct, and whatever
> problems you are saying there are by lying about the hard header len are
> dwarfed by the problems caused by the inability to store the ll address
> anywhere between hard_header and send time.
IB wants to pass addressing information between layers, it needs to
find a safe way to do that. The currently propsoed patch does not
meet this criteria.
Pushing metadata before the head of the SKB data pointer is illegal,
as the layers in between might want to push protocol headers, mirror
the packet to another interface, etc.
So this "metadata in SKB data" approach is buggy too.
^ permalink raw reply
* RE: Introduction of libqedr to the Consolidated Userspace RDMA Library Repo
From: Amrani, Ram @ 2016-10-13 14:37 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Elior, Ariel,
Kalderon, Michal, Borundia, Rajesh,
dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
In-Reply-To: <SN1PR07MB2207B8BA2C9C9A1FC610DE9FF8D90-mikhvbZlbf8TSoR2DauN2+FPX92sqiQdvxpqHgZTriW3zl9H0oFU5g@public.gmane.org>
> >You will want to look at the various patches I've prepared and ensure
> >you cover off the basic cleanups that have already been done, and that
> >your code compiles warning-free on FC24.
By cleanups, do you mean to make sure the code compiles warning-free under
the various flags mentioned throughout the git log (-Wempty-body and etc.),
or are there other kinds of cleanups that I'm missing?
> >Once you feel everything is ready then post it to the mailing list and
> >send a pull request. For the mailing list you can just split the
> >patches by file..
As we don't have a publically accessed server I cannot do this at the moment (I'm checking how we can set one up).
Is it OK if I send you the update as a series of patches?
--
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: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: Doug Ledford @ 2016-10-13 14:35 UTC (permalink / raw)
To: David Miller, pabeni-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161013.102432.1450901395774429343.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1817 bytes --]
On 10/13/2016 10:24 AM, David Miller wrote:
> From: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Date: Tue, 11 Oct 2016 19:15:44 +0200
>
>> After the commit 9207f9d45b0a ("net: preserve IP control block
>> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
>> That destroy the IPoIB address information cached there,
>> causing a severe performance regression, as better described here:
>>
>> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
>>
>> This change moves the data cached by the IPoIB driver from the
>> skb control lock into the IPoIB hard header, as done before
>> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
>> and use skb->cb to stash LL addresses").
>> In order to avoid GRO issue, on packet reception, the IPoIB driver
>> stash into the skb a dummy pseudo header, so that the received
>> packets have actually a hard header matching the declared length.
>> Also the connected mode maximum mtu is reduced by 16 bytes to
>> cope with the increased hard header len.
>>
>> After this commit, IPoIB performances are back to pre-regression
>> value.
>>
>> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
>> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> Not providing an accurate hard_header_len causes many problems.
>
> In fact we recently fixed the mlxsw driver to stop doing this.
>
Sure, but there are too many users of the cb struct, and whatever
problems you are saying there are by lying about the hard header len are
dwarfed by the problems caused by the inability to store the ll address
anywhere between hard_header and send time.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 00/11] net: Fix netdev adjacency tracking
From: David Ahern @ 2016-10-13 14:32 UTC (permalink / raw)
To: Jiri Pirko
Cc: jiri-VPRAkNaXOzVWk0Htik3J/w, netdev-u79uwXL29TY76Z2rM5mHXA,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
j.vosburgh-Re5JQEeQqe8AvxtiuMwx3w, vfalico-Re5JQEeQqe8AvxtiuMwx3w,
andy-QlMahl40kYEqcZcGjlUOXw,
jeffrey.t.kirsher-ral2JQCrhuEAvxtiuMwx3w,
intel-wired-lan-qjLDD68F18P21nG7glBr7A
In-Reply-To: <20161013073424.GB1816-6KJVSR23iU488b5SBfVpbw@public.gmane.org>
On 10/13/16 1:34 AM, Jiri Pirko wrote:
>
> Although I didn't like the "all-list" idea when Veaceslav pushed it
> because it looked to me like a big hammer, it turned out to be very handy
> and quick for traversing neighbours. Why it cannot be fixed?
>
> The walks with possibly hundreds of function calls instead of a single
> list traverse worries me.
>
I have been looking at this code for a week now. Every solution I came up with that solved the original problems introduced new ones -- usually with adjacency remove expecting an adjacency that did not exist. In the end I came to the conclusion it is not possible to maintain an all_adj_list when the upper tree can have multiple paths to the top device. If someone thinks otherwise I am happy to test a patch.
--
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: [PATCH] IB/ipoib: move back the IB LL address into the hard header
From: David Miller @ 2016-10-13 14:24 UTC (permalink / raw)
To: pabeni-H+wXaHxf7aLQT0dZR+AlfA
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
sean.hefty-ral2JQCrhuEAvxtiuMwx3w,
hal.rosenstock-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1dbd83dfe7f435eecc5bc460e901b47758280f30.1476206016.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
From: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Tue, 11 Oct 2016 19:15:44 +0200
> After the commit 9207f9d45b0a ("net: preserve IP control block
> during GSO segmentation"), the GSO CB and the IPoIB CB conflict.
> That destroy the IPoIB address information cached there,
> causing a severe performance regression, as better described here:
>
> http://marc.info/?l=linux-kernel&m=146787279825501&w=2
>
> This change moves the data cached by the IPoIB driver from the
> skb control lock into the IPoIB hard header, as done before
> the commit 936d7de3d736 ("IPoIB: Stop lying about hard_header_len
> and use skb->cb to stash LL addresses").
> In order to avoid GRO issue, on packet reception, the IPoIB driver
> stash into the skb a dummy pseudo header, so that the received
> packets have actually a hard header matching the declared length.
> Also the connected mode maximum mtu is reduced by 16 bytes to
> cope with the increased hard header len.
>
> After this commit, IPoIB performances are back to pre-regression
> value.
>
> Fixes: 9207f9d45b0a ("net: preserve IP control block during GSO segmentation")
> Signed-off-by: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Not providing an accurate hard_header_len causes many problems.
In fact we recently fixed the mlxsw driver to stop doing this.
--
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: [PATCH rdma-next 00/10] Hardware tag matching support
From: Doug Ledford @ 2016-10-13 14:15 UTC (permalink / raw)
To: Hefty, Sean, Leon Romanovsky
Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1828884A29C6694DAF28B7E6B8A82373AB093986-P5GAC/sN6hkd3b2yrw5b5LfspsVTdybXVpNB7YpNyf8@public.gmane.org>
[-- Attachment #1.1: Type: text/plain, Size: 1441 bytes --]
On 10/7/2016 12:47 PM, Hefty, Sean wrote:
>> For any reasons, I don't see this patch set in your tree. Did I miss
>> it?
>>
>> Thanks
>>
>>>
>>> Thanks,
>>> Artemy & Leon
>>>
>>> Artemy Kovalyov (10):
>>> IB/core: Add XRQ capabilities
>>> IB/core: Make CQ separate part of SRQ context
>>> IB/core: Add new SRQ type IB_SRQT_TAG_MATCHING
>>> IB/uverbs: Expose tag matching capabilties to UAPI
>>> IB/uverbs: Expose XRQ capabilities
>>> IB/uverbs: Add XRQ creation parameter to UAPI
>>> IB/uverbs: Add new SRQ type IB_SRQT_TAG_MATCHING
>>> IB/mlx5: Fill XRQ capabilities
>>> net/mlx5: Add XRQ support
>>> IB/mlx5: Support IB_SRQT_TAG_MATCHING
>
> I was out when these were submitted, so this may have been answered. These patches change the uABI. Have the changes been vetted out by the IBTA or some other standards (I'm using that term loosely) organization?
>
I doubt they have, but they change the kernel to user library API. I'm
not so worried about that since it's entirely under our control. It's
the user library to application API that I would be more concerned with.
As these stand, they look fairly clean to me. Minimal in disruption and
I see no reason other vendors couldn't use this to do tag matching on
their own hardware if they wanted. I'm inclined to take them.
--
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* Re: [bug report] IB/hns: Add driver files for hns RoCE driver
From: oulijun @ 2016-10-13 13:18 UTC (permalink / raw)
To: Dan Carpenter, Doug Ledford; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161013114341.GA8275@mwanda>
在 2016/10/13 19:43, Dan Carpenter 写道:
> Hello oulijun,
>
> The patch 9a4435375cd1: "IB/hns: Add driver files for hns RoCE
> driver" from Jul 21, 2016, leads to the following static checker
> warning:
>
> drivers/infiniband/hw/hns/hns_roce_mr.c:575 hns_roce_reg_user_mr()
> warn: no lower bound on 'n'
>
> drivers/infiniband/hw/hns/hns_roce_mr.c
> 542 struct ib_mr *hns_roce_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
> 543 u64 virt_addr, int access_flags,
> 544 struct ib_udata *udata)
> 545 {
> 546 struct hns_roce_dev *hr_dev = to_hr_dev(pd->device);
> 547 struct device *dev = &hr_dev->pdev->dev;
> 548 struct hns_roce_mr *mr = NULL;
> 549 int ret = 0;
> 550 int n = 0;
> ^^^^^^^^^
>
> Notice this is signed. Please don't initialize variables to bogus
> values. The compiler has a feature to warn about uninitialized
> variables but by assigning bogus valus to "n" then you are disabling the
> safety checks and potentially hiding bugs.
Thanks, we will fix it in next patch!
>
> 551
> 552 mr = kmalloc(sizeof(*mr), GFP_KERNEL);
> 553 if (!mr)
> 554 return ERR_PTR(-ENOMEM);
> 555
> 556 mr->umem = ib_umem_get(pd->uobject->context, start, length,
> 557 access_flags, 0);
> 558 if (IS_ERR(mr->umem)) {
> 559 ret = PTR_ERR(mr->umem);
> 560 goto err_free;
> 561 }
> 562
> 563 n = ib_umem_page_count(mr->umem);
>
> Depending on the config then ib_umem_page_count() can return -EINVAL.
> Probably it's not possible here. Anyway, probably the right thing is
> to check:
>
> if (n < 0) {
> ret = -EINVAL;
> goto umem;
> }
>
Thanks for your reviewing. I have checked the ib_umem_page_count(), Maybe it
will not return the -EINVAL
when configured CONFIG_INFINIBAND_USER_MEM, the function is
int ib_umem_page_count(struct ib_umem *umem)
{
int shift;
int i;
int n;
struct scatterlist *sg;
if (umem->odp_data)
return ib_umem_num_pages(umem);
shift = ilog2(umem->page_size);
n = 0;
for_each_sg(umem->sg_head.sgl, sg, umem->nmap, i)
n += sg_dma_len(sg) >> shift;
return n;
}
EXPORT_SYMBOL(ib_umem_page_count);
when not configured CONFIG_INFINIBAND_USER_MEM, the function as follow:
static inline int ib_umem_page_count(struct ib_umem *umem) { return 0; }
> It silences the static checker warning.
>
> 564 if (mr->umem->page_size != HNS_ROCE_HEM_PAGE_SIZE) {
> 565 dev_err(dev, "Just support 4K page size but is 0x%x now!\n",
> 566 mr->umem->page_size);
>
> Should we continue here or is there supposed to be a goto umem;?
Thanks, Dan Carpenter
We have taken notice of the problem and have sent a patch to Doug to fix last two problem. the patch link as follow:
https://patchwork.kernel.org/patch/9342015/
>
> 567 }
> 568
> 569 if (n > HNS_ROCE_MAX_MTPT_PBL_NUM) {
> 570 dev_err(dev, " MR len %lld err. MR is limited to 4G at most!\n",
> 571 length);
>
>
> We should be setting "ret = -EINVAL;" here.
Thanks, Dan Carpenter
We have taken notice of the problem and have sent a patch to Doug to fix last two problem. the patch link as follow:
https://patchwork.kernel.org/patch/9342015/
>
> 572 goto err_umem;
> 573 }
> 574
> 575 ret = hns_roce_mr_alloc(hr_dev, to_hr_pd(pd)->pdn, virt_addr, length,
> 576 access_flags, n, mr);
> 577 if (ret)
> 578 goto err_umem;
> 579
>
> regards,
> dan carpenter
>
> .
>
--
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: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Doug Ledford @ 2016-10-13 11:57 UTC (permalink / raw)
To: Salil Mehta, David Miller
Cc: Zhuangyuzeng (Yisen), Huwei (Xavier), oulijun,
mehta.salil.lnk@gmail.com, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Linuxarm
In-Reply-To: <F4CC6FACFEB3C54C9141D49AD221F7F91A74F326@lhreml503-mbx>
[-- Attachment #1.1: Type: text/plain, Size: 1595 bytes --]
On 10/13/2016 7:26 AM, Salil Mehta wrote:
>> -----Original Message-----
>> From: David Miller [mailto:davem@davemloft.net]
>> Sent: Friday, September 30, 2016 6:34 AM
>> To: Salil Mehta
>> Cc: dledford@redhat.com; Zhuangyuzeng (Yisen); Huwei (Xavier); oulijun;
>> mehta.salil.lnk@gmail.com; linux-rdma@vger.kernel.org;
>> netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
>> Subject: Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in
>> HNS driver
>>
>> From: Salil Mehta <salil.mehta@huawei.com>
>> Date: Thu, 29 Sep 2016 18:09:08 +0100
>>
>>> This patch-set introduces fix to some Bugs, potential problems
>>> and code improvements identified during internal review and
>>> testing of Hisilicon Network Subsystem driver.
>>>
>>> Submit Change
>>> V1->V2: This addresses the feedbacks provided by David Miller
>>> and Doug Ledford
>>
>> So Doug my understanding is if this makes it through review
> Hi Dave,
> A gentle reminder regarding this. I was wondering if you think these
> have been reviewed enough and by any chance these patches can be included
> in current 4.9 merge window. This will save us a lot of effort.
>
> As I understand Doug is waiting for your review approval on this
> patch-set.
I am, and I was just getting ready to ask again today myself.
> Thanks in anticipation
> Best regards
> Salil
>> this is going to be merged into your tree, you prepare a
>> branch for me, and then I pull from that?
>>
>> Thanks in advance.
--
Doug Ledford <dledford@redhat.com>
GPG Key ID: 0E572FDD
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 884 bytes --]
^ permalink raw reply
* [bug report] IB/hns: Add driver files for hns RoCE driver
From: Dan Carpenter @ 2016-10-13 11:43 UTC (permalink / raw)
To: oulijun-hv44wF8Li93QT0dZR+AlfA; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA
Hello oulijun,
The patch 9a4435375cd1: "IB/hns: Add driver files for hns RoCE
driver" from Jul 21, 2016, leads to the following static checker
warning:
drivers/infiniband/hw/hns/hns_roce_mr.c:575 hns_roce_reg_user_mr()
warn: no lower bound on 'n'
drivers/infiniband/hw/hns/hns_roce_mr.c
542 struct ib_mr *hns_roce_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
543 u64 virt_addr, int access_flags,
544 struct ib_udata *udata)
545 {
546 struct hns_roce_dev *hr_dev = to_hr_dev(pd->device);
547 struct device *dev = &hr_dev->pdev->dev;
548 struct hns_roce_mr *mr = NULL;
549 int ret = 0;
550 int n = 0;
^^^^^^^^^
Notice this is signed. Please don't initialize variables to bogus
values. The compiler has a feature to warn about uninitialized
variables but by assigning bogus valus to "n" then you are disabling the
safety checks and potentially hiding bugs.
551
552 mr = kmalloc(sizeof(*mr), GFP_KERNEL);
553 if (!mr)
554 return ERR_PTR(-ENOMEM);
555
556 mr->umem = ib_umem_get(pd->uobject->context, start, length,
557 access_flags, 0);
558 if (IS_ERR(mr->umem)) {
559 ret = PTR_ERR(mr->umem);
560 goto err_free;
561 }
562
563 n = ib_umem_page_count(mr->umem);
Depending on the config then ib_umem_page_count() can return -EINVAL.
Probably it's not possible here. Anyway, probably the right thing is
to check:
if (n < 0) {
ret = -EINVAL;
goto umem;
}
It silences the static checker warning.
564 if (mr->umem->page_size != HNS_ROCE_HEM_PAGE_SIZE) {
565 dev_err(dev, "Just support 4K page size but is 0x%x now!\n",
566 mr->umem->page_size);
Should we continue here or is there supposed to be a goto umem;?
567 }
568
569 if (n > HNS_ROCE_MAX_MTPT_PBL_NUM) {
570 dev_err(dev, " MR len %lld err. MR is limited to 4G at most!\n",
571 length);
We should be setting "ret = -EINVAL;" here.
572 goto err_umem;
573 }
574
575 ret = hns_roce_mr_alloc(hr_dev, to_hr_pd(pd)->pdn, virt_addr, length,
576 access_flags, n, mr);
577 if (ret)
578 goto err_umem;
579
regards,
dan carpenter
--
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: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in HNS driver
From: Salil Mehta @ 2016-10-13 11:26 UTC (permalink / raw)
To: David Miller
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org,
Zhuangyuzeng (Yisen), Huwei (Xavier), oulijun,
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm
In-Reply-To: <20160930.013341.118711534087556597.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> -----Original Message-----
> From: David Miller [mailto:davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org]
> Sent: Friday, September 30, 2016 6:34 AM
> To: Salil Mehta
> Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zhuangyuzeng (Yisen); Huwei (Xavier); oulijun;
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> Subject: Re: [PATCH V2 for-next 0/8] Bug Fixes and Code Improvement in
> HNS driver
>
> From: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Date: Thu, 29 Sep 2016 18:09:08 +0100
>
> > This patch-set introduces fix to some Bugs, potential problems
> > and code improvements identified during internal review and
> > testing of Hisilicon Network Subsystem driver.
> >
> > Submit Change
> > V1->V2: This addresses the feedbacks provided by David Miller
> > and Doug Ledford
>
> So Doug my understanding is if this makes it through review
Hi Dave,
A gentle reminder regarding this. I was wondering if you think these
have been reviewed enough and by any chance these patches can be included
in current 4.9 merge window. This will save us a lot of effort.
As I understand Doug is waiting for your review approval on this
patch-set.
Thanks in anticipation
Best regards
Salil
> this is going to be merged into your tree, you prepare a
> branch for me, and then I pull from that?
>
> Thanks in advance.
--
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: [PATCH 5/5] rxe_cfg: Use printf() instead of using a loop
From: Yonatan Cohen @ 2016-10-13 11:07 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Jason Gunthorpe; +Cc: Moni Shoua, linux-rdma
In-Reply-To: <278745ec-1e31-4379-27d9-40f371d30c29-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> providers/rxe/rxe_cfg | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 0a3dcf16957c..6c414fbda6fd 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -241,7 +241,6 @@ sub status_print {
> my @flen = ();
> my $num_fields = 0;
> my $i;
> - my $j;
> my $pad;
> my $line;
>
> @@ -281,9 +280,7 @@ sub status_print {
> else {
> print " ";
> }
> - for ($j = 0; $j < $pad; $j++) {
> - print " ";
> - }
> + printf("%*s", $pad, "");
> }
> print "\n";
> }
>
ack
--
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: [PATCH 4/5] rxe_cfg: Remove dead code from show_module_status()
From: Yonatan Cohen @ 2016-10-13 11:06 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Jason Gunthorpe; +Cc: Moni Shoua, linux-rdma
In-Reply-To: <beb18346-316e-5d71-dce2-091d700797d6-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> providers/rxe/rxe_cfg | 18 +-----------------
> 1 file changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index a4e9c0407a8b..0a3dcf16957c 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -300,23 +300,7 @@ sub check_module_status {
>
> # print driver load status and ethertype for rdma_rxe and rdma_rxe_net
> sub show_module_status {
> - my $rxe_loaded = 1;
> -
> - if (!(-e $sys)) {
> - $rxe_loaded = 0;
> - }
> -
> - if ($rxe_loaded) {
> - return 0;
> - }
> - elsif (!$rxe_loaded) {
> - print "rxe modules not loaded\n";
> - return 1;
> - }
> - else {
> - print "Configuration does not make sense\n";
> - return 1;
> - }
> + print "rdma_rxe module not loaded\n" if (!(-e $sys));
> }
>
> # print rxe status
>
ack
--
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: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Parav Pandit @ 2016-10-13 11:04 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <20161013103430.GB9282-2ukJVAZIZ/Y@public.gmane.org>
Hi Leon,
On Thu, Oct 13, 2016 at 4:04 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Oct 10, 2016 at 07:02:11PM +0530, Parav Pandit wrote:
>> Hi Tejun,
>>
>> On Mon, Oct 10, 2016 at 6:50 PM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > Hello, Parav.
>> >
>> > On Mon, Oct 10, 2016 at 06:43:59PM +0530, Parav Pandit wrote:
>> >> > Also, I don't get what you mean by using percentage and when people
>> >> > brought up this idea, it always has been stemming from
>> >> > misunderstanding. Can you please elaborate how percentage based
>> >> > proportional control would work? What would 100% mean when cgroups
>> >> > can come and go?
>> >>
>> >> When 100% is given to one cgroup, all resources of all type can be
>> >> charged by processes of that cgroup.
>> >> Resources are stateful resource. So when cgroup goes away, they go
>> >> back to global pool (or hw).
>> >> Giving 100% to two cgroups is configuration error anyway (or without config).
>> >
>> > That isn't proportional control. That's using percentage as the unit
>> > to implement absolute limits. Proportional control implies work
>> > conservation.
>> >
>> >> As you know weight configuration allows automatic increase/decrease of
>> >> resource to other cgroups when one of them go away, as opposed to
>> >> absolute value. How this is going to work in exact terms for stateful
>> >
>> > Hmm.... so are you saying that ti'd be work-conserving?
>> They cannot be work conversing.
>>
>> > But what does
>> > it mean to say "30%" and then have it all resources when there are no
>> > other users. Also, is it even possible to take back what have already
>> > been allocated and are in use?
>> >
>> Most resources that I know of, and whats described in current
>> cgroup_rdma.h are not work conversing, therefore it cannot be taken
>> back.
>>
>> >> Nop. Thats not true.
>> >> (a) Every new resource has to be defined in cgroup_rdma.h
>> >> (b) charge()/uncharge() has to happen by the cgroup for each.
>> >> (c) Letting drivers do will make things fall apart. There are no APIs
>> >> exposed either to let drivers know process cgroup either. There is no
>> >> intention either.
>> >>
>> >> (d) ratio means -if adapter has
>> >> 100 resources of type - A,
>> >> 80 resource of type - B,
>> >>
>> >> 10% for cgroup-1 means,
>> >> 10 resource of type - A
>> >> 8 resource of type - B
>> >
>> > So, this is not work-conserving. There's too much confusion here.
>>
>> Give me some more time, I will think more and take feeback from Leon
>> and others on
>> (a) how can we implement or want to implement weight like
>> functionality for non-work-conversing resource
>
> I'm not fully understand the question. RDMA resources are static and
> won't be recalculated dynamically for running cgroups consumers while
> new cgroup is started. In this situation, weights and percentages are
> the same.
>
Let me try again to take weights example.
total resources distributed is based on equation:
resource_of_cg = weight_of_cg * max_resource_of_hca / (sum of all weights)
say there is only one cg-A. Lets say it has weight of 20.
max_resource_of_hca = 100.
So total resource_for_cg_A = (20 * 100) / 20 = 100 (all resources).
Now new cg-B is created with weight of 20.
So with new cg-B, cg-A and cg-B will get 50 resources.
With cg-C creation with weight of 20, each cg gets 33 resources.
Which means that resources are recalculated dynamically on
running/creating new cgroups.
>> (b) what could be its acceptable limitations of that interface would be
>> before we propose you.
>
> More easy is to summarize requirements:
> 1. It needs to be convenient for users.
> 2. It can limit any future objects without change in user tools.
Why don't we have such requirements on the actual dataplane and
control plane APIs side (similar to having abstract socket APIs).
Instead we expect applications to change to make use of new verbs
objects for performance, functionality etc.
New future objects can be limited if we introduce weights/percentage
knob but at the cost of not able to tune for performance.
Usually end-users will use application templates when they deploy for
specific applications, such as mongodb, MPI cluster, glusterFS cluster
etc.
So those application specific plug-in would program exact ratio of MR
to QP or PD to QP etc by writing values to rdma.max depending on MPI
rank, cluster size etc.
weights API allow to auto adjust the value for existing cgroups when
new cgroups are added/removed when deployed application is not well
defined.
>
>>
>> At minimum we would need to expose actual value in rdma.max in
>> subsequent patch, instead of exposing just "max" string. I don't want
>> to complicate this discussion but similar functionality is needed for
>> pid controller as well to expose actual value.
>>
>> >
>> > Thanks.
>> >
>> > --
>> > tejun
--
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: [PATCH 3/5] rxe_cfg: Do not suppress stderr
From: Yonatan Cohen @ 2016-10-13 11:00 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Jason Gunthorpe; +Cc: Moni Shoua, linux-rdma
In-Reply-To: <b97aebd2-37fc-c526-0bf6-438c194111a1-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/7/2016 9:39 PM, Bart Van Assche wrote:
> Suppressing stderr if stdout is already redirected is wrong. Hence
> do not suppress stderr. If e.g. the ib_uverbs kernel module is not
> loaded, without this patch no error messages are printed. With this
> patch applied the following is printed:
>
> Failed to get IB devices list: Function not implemented
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> providers/rxe/rxe_cfg | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 1e6a5db2f2b8..a4e9c0407a8b 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -39,7 +39,6 @@ use File::Basename;
> use Getopt::Long;
> use Switch;
>
> -my $z = "2>/dev/null";
> my $help = 0;
> my $no_persist = 0;
> my $debug = 0;
> @@ -150,7 +149,7 @@ sub get_dev_info {
>
> $eth_list[$i++] = $eth;
>
> - @lines = `ethtool -i $eth $z`;
> + @lines = `ethtool -i $eth`;
> foreach $line (@lines) {
> chomp($line);
>
> @@ -171,7 +170,7 @@ sub get_dev_info {
> $link_state{$eth} = "";
> $link_speed{$eth} = "";
>
> - @lines = `ethtool $eth $z`;
> + @lines = `ethtool $eth`;
> foreach $line (@lines) {
> chomp($line);
>
> @@ -190,7 +189,7 @@ sub get_dev_info {
> $ipv4_addr{$eth} = " ";
> $eth_mtu{$eth} = "";
>
> - @lines = `ifconfig $eth $z`;
> + @lines = `ifconfig $eth`;
> foreach $line (@lines) {
> # get IP address
> if ($line =~ /inet addr/) {
> @@ -211,7 +210,7 @@ sub get_dev_info {
> # get rxe mtu
> foreach my $rxe (@rxe_array) {
>
> - @lines = `ibv_devinfo -d $rxe $z`;
> + @lines = `ibv_devinfo -d $rxe`;
> foreach $line (@lines) {
> if ($line =~ "active_mtu") {
> $line =~ s/^\s+active_mtu:\s+//g;
> @@ -494,7 +493,7 @@ sub get_devinfo {
> my $rxe = $_[0];
>
> my $cmd = "ibv_devinfo -d $rxe";
> - return `$cmd $z`;
> + return `$cmd`;
> }
>
> # allow unsupported modules to load in SLES11 if allowed
> @@ -679,7 +678,7 @@ sub main {
> # create persistence file if necessary
> mkdir -p $persistence_path;
> if (!(-e $persistence_file)) {
> - `touch $persistence_file $z`;
> + `touch $persistence_file`;
> }
>
> # Get full context of the configuration
>
Ack
--
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: [PATCH 2/5] rxe_cfg: Initialize $rxe_mtu even if ibv_definfo fails
From: Yonatan Cohen @ 2016-10-13 11:00 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Jason Gunthorpe; +Cc: Moni Shoua, linux-rdma
In-Reply-To: <298d9c50-821d-b3f6-cfb3-366ad919c025-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/7/2016 9:38 PM, Bart Van Assche wrote:
> This patch avoids that the following error message is printed if
> ibv_devinfo fails:
>
> Use of uninitialized value $rmtu in string at /usr/local/bin/rxe_cfg line 364.
>
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> providers/rxe/rxe_cfg | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 0310d25bad91..1e6a5db2f2b8 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -220,6 +220,7 @@ sub get_dev_info {
> $rxe_mtu{$rxe} = $line;
> }
> }
> + $rxe_mtu{$rxe} = "(?)" if (!$rxe_mtu{$rxe});
> }
> }
>
>
Ack
--
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: [PATCH 1/5] rxe_cfg: Use Perl functions instead of parsing the output of shell commands
From: Yonatan Cohen @ 2016-10-13 10:53 UTC (permalink / raw)
To: Bart Van Assche, Doug Ledford, Jason Gunthorpe; +Cc: Moni Shoua, linux-rdma
In-Reply-To: <51c02d54-9189-2993-a9cb-a6f6452e9ff3-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
On 10/7/2016 9:38 PM, Bart Van Assche wrote:
> Signed-off-by: Bart Van Assche <bart.vanassche-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org>
> ---
> providers/rxe/rxe_cfg | 67 ++++++++++++++++++++++++---------------------------
> 1 file changed, 31 insertions(+), 36 deletions(-)
>
> diff --git a/providers/rxe/rxe_cfg b/providers/rxe/rxe_cfg
> index 6783dc78c340..0310d25bad91 100755
> --- a/providers/rxe/rxe_cfg
> +++ b/providers/rxe/rxe_cfg
> @@ -35,6 +35,7 @@
> use warnings;
> use strict;
>
> +use File::Basename;
> use Getopt::Long;
> use Switch;
>
> @@ -67,20 +68,25 @@ my @mlx4_port;
> my @mlx4_ether;
> my @roce_list;
>
> +# Read a file and return its contents as a string.
> +sub read_file {
> + my $filename = shift;
> + my $result = "";
> +
> + if (open(FILE, $filename)) {
> + $result = <FILE>;
> + close FILE;
> + }
> + return $result;
> +}
> +
> #get mapping between rxe and eth devices
> sub get_names {
> - return if (!(-e "/sys/class/infiniband"));
> -
> - my @list;
> - my $rxe;
> - my $eth;
> my $i = 0;
>
> - @list = `ls /sys/class/infiniband | grep rxe $z`;
> - foreach $rxe (@list) {
> - chomp($rxe);
> -
> - $eth = `cat /sys/class/infiniband/$rxe/parent $z`;
> + foreach my $rxe (glob("/sys/class/infiniband/rxe*")) {
> + $rxe = basename($rxe);
> + my $eth = read_file("/sys/class/infiniband/$rxe/parent");
> chomp($eth);
>
> if (($eth =~ /[\w]+[\d]/)
> @@ -98,25 +104,13 @@ sub get_names {
>
> # get list of Mellanox RoCE ports
> sub get_mlx4_list {
> - return if (!(-e "/sys/class/infiniband"));
> -
> - my @list;
> - my $mlx4;
> - my @ports;
> - my $port;
> - my $link;
> my $i = 0;
>
> - @list = `ls /sys/class/infiniband | grep mlx4_ $z`;
> - foreach $mlx4 (@list) {
> - chomp($mlx4);
> - chdir("/sys/class/infiniband/$mlx4/ports");
> -
> - @ports = `ls $z`;
> - foreach $port (@ports) {
> - chomp($port);
> -
> - $link= `cat $port/link_layer $z`;
> + foreach my $mlx4 (glob("/sys/class/infiniband/mlx4_*")) {
> + $mlx4 = basename($mlx4);
> + foreach my $port (glob("/sys/class/infiniband/$mlx4/ports/*")) {
> + $port = basename($port);
> + my $link = read_file("$port/link_layer");
> chomp($link);
>
> if ($link =~ "Ethernet") {
> @@ -141,10 +135,10 @@ sub get_dev_info {
> get_mlx4_list();
>
> my @my_eth_list = ();
> - my @my_eth_devs = `ls /sys/class/net`;
> - foreach my $my_eth_dev (@my_eth_devs) {
> - chomp($my_eth_dev);
> - my $my_dev_type = `cat /sys/class/net/${my_eth_dev}/type`;
> + foreach my $my_eth_dev (glob("/sys/class/net/*")) {
> + $my_eth_dev = basename($my_eth_dev);
> + my $my_dev_type = read_file("/sys/class/net/${my_eth_dev}/type");
> + chomp($my_dev_type);
> if ($my_dev_type == "1") {
> push(@my_eth_list, "$my_eth_dev");
> }
> @@ -378,11 +372,11 @@ sub do_status {
>
> # read file containing list of ethernet devices into a list
> sub populate_persistence {
> - my @lines = `cat $persistence_file $z`;
> - my $line;
> my $i = 0;
>
> - foreach $line (@lines) {
> + open FILE, $persistence_file;
> + while(<FILE>) {
> + my $line = $_;
> chomp($line);
> $line =~ s/^\s+//g;
> if ($line =~ /[\w]+[\d]/) {
> @@ -394,6 +388,7 @@ sub populate_persistence {
> }
> }
> }
> + close FILE;
>
> $num_persistent = $i;
> }
> @@ -604,7 +599,7 @@ sub do_debug {
> }
> }
>
> - my $current = `cat $debugfile $z`;
> + my $current = read_file($debugfile);
> chomp($current);
> if ($current > 0) {
> print "Debug is ON ($current)\n";
> @@ -681,7 +676,7 @@ sub main {
> }
>
> # create persistence file if necessary
> - `mkdir -p $persistence_path $z`;
> + mkdir -p $persistence_path;
> if (!(-e $persistence_file)) {
> `touch $persistence_file $z`;
> }
>
ack
--
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: 【For help】 some tips for upstream the userspace driver code of hns_roce
From: oulijun @ 2016-10-13 10:39 UTC (permalink / raw)
To: Leon Romanovsky; +Cc: Doug Ledford, Linuxarm, linux-rdma
In-Reply-To: <20161013093659.GA9282-2ukJVAZIZ/Y@public.gmane.org>
在 2016/10/13 17:36, Leon Romanovsky 写道:
> On Thu, Oct 13, 2016 at 04:09:46PM +0800, oulijun wrote:
>> Hi, Doug Ledford
>> Now, I am preparing for upstreaming the userspace driver code of hns_roce. But, we have
>> some difficulty how to upstream it. for example,
>> i) which branch of ofed should we upstream ?
>> ii) if want to upstream the userspace driver of hns_roce successfully, how we should do?
>>
>> Can you give me some guides?
>
> Hi Lijun,
>
> Our main library for all user-space stack is located at github [1].
> Please see similar question from Ram [2] and Jason's response [3] to it.
>
> [1] github.com/linux-rdma/rdma-core
> [2[ http://marc.info/?l=linux-rdma&m=147574131706820&w=2
> [3] http://marc.info/?l=linux-rdma&m=147577168016989&w=2
>
> Thanks.
>
>>
>> thanks
>> Lijun Ou
>>
Hi, leon
Thank your reply. I will learn it careful.
thanks
Lijun Ou
>> --
>> 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
--
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: [PATCHv12 0/3] rdmacg: IB/core: rdma controller support
From: Leon Romanovsky @ 2016-10-13 10:34 UTC (permalink / raw)
To: Parav Pandit
Cc: Tejun Heo, cgroups-u79uwXL29TY76Z2rM5mHXA, linux-rdma, Li Zefan,
Johannes Weiner, Doug Ledford, Christoph Hellwig, Liran Liss,
Hefty, Sean, Jason Gunthorpe, Haggai Eran,
james.l.morris-QHcLZuEGTsvQT0dZR+AlfA, Or Gerlitz, Matan Barak
In-Reply-To: <CAG53R5ULKCqtw45E6t4hYdRV+y_OQqVazf=7A7Ax_XAJ2K0_dw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3281 bytes --]
On Mon, Oct 10, 2016 at 07:02:11PM +0530, Parav Pandit wrote:
> Hi Tejun,
>
> On Mon, Oct 10, 2016 at 6:50 PM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > Hello, Parav.
> >
> > On Mon, Oct 10, 2016 at 06:43:59PM +0530, Parav Pandit wrote:
> >> > Also, I don't get what you mean by using percentage and when people
> >> > brought up this idea, it always has been stemming from
> >> > misunderstanding. Can you please elaborate how percentage based
> >> > proportional control would work? What would 100% mean when cgroups
> >> > can come and go?
> >>
> >> When 100% is given to one cgroup, all resources of all type can be
> >> charged by processes of that cgroup.
> >> Resources are stateful resource. So when cgroup goes away, they go
> >> back to global pool (or hw).
> >> Giving 100% to two cgroups is configuration error anyway (or without config).
> >
> > That isn't proportional control. That's using percentage as the unit
> > to implement absolute limits. Proportional control implies work
> > conservation.
> >
> >> As you know weight configuration allows automatic increase/decrease of
> >> resource to other cgroups when one of them go away, as opposed to
> >> absolute value. How this is going to work in exact terms for stateful
> >
> > Hmm.... so are you saying that ti'd be work-conserving?
> They cannot be work conversing.
>
> > But what does
> > it mean to say "30%" and then have it all resources when there are no
> > other users. Also, is it even possible to take back what have already
> > been allocated and are in use?
> >
> Most resources that I know of, and whats described in current
> cgroup_rdma.h are not work conversing, therefore it cannot be taken
> back.
>
> >> Nop. Thats not true.
> >> (a) Every new resource has to be defined in cgroup_rdma.h
> >> (b) charge()/uncharge() has to happen by the cgroup for each.
> >> (c) Letting drivers do will make things fall apart. There are no APIs
> >> exposed either to let drivers know process cgroup either. There is no
> >> intention either.
> >>
> >> (d) ratio means -if adapter has
> >> 100 resources of type - A,
> >> 80 resource of type - B,
> >>
> >> 10% for cgroup-1 means,
> >> 10 resource of type - A
> >> 8 resource of type - B
> >
> > So, this is not work-conserving. There's too much confusion here.
>
> Give me some more time, I will think more and take feeback from Leon
> and others on
> (a) how can we implement or want to implement weight like
> functionality for non-work-conversing resource
I'm not fully understand the question. RDMA resources are static and
won't be recalculated dynamically for running cgroups consumers while
new cgroup is started. In this situation, weights and percentages are
the same.
> (b) what could be its acceptable limitations of that interface would be
> before we propose you.
More easy is to summarize requirements:
1. It needs to be convenient for users.
2. It can limit any future objects without change in user tools.
>
> At minimum we would need to expose actual value in rdma.max in
> subsequent patch, instead of exposing just "max" string. I don't want
> to complicate this discussion but similar functionality is needed for
> pid controller as well to expose actual value.
>
> >
> > Thanks.
> >
> > --
> > tejun
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: 【For help】 some tips for upstream the userspace driver code of hns_roce
From: Leon Romanovsky @ 2016-10-13 9:36 UTC (permalink / raw)
To: oulijun; +Cc: Doug Ledford, Linuxarm, linux-rdma
In-Reply-To: <57FF414A.3000200-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 964 bytes --]
On Thu, Oct 13, 2016 at 04:09:46PM +0800, oulijun wrote:
> Hi, Doug Ledford
> Now, I am preparing for upstreaming the userspace driver code of hns_roce. But, we have
> some difficulty how to upstream it. for example,
> i) which branch of ofed should we upstream ?
> ii) if want to upstream the userspace driver of hns_roce successfully, how we should do?
>
> Can you give me some guides?
Hi Lijun,
Our main library for all user-space stack is located at github [1].
Please see similar question from Ram [2] and Jason's response [3] to it.
[1] github.com/linux-rdma/rdma-core
[2[ http://marc.info/?l=linux-rdma&m=147574131706820&w=2
[3] http://marc.info/?l=linux-rdma&m=147577168016989&w=2
Thanks.
>
> thanks
> Lijun Ou
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ 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