Re: [PATCH] IB/IPoIB: Fix kernel panic on multicast flow
On Thu, 7 Jan 2016, Erez Shitrit wrote: > ipoib_mcast_restart_task calls ipoib_mcast_remove_list with the > parameter mcast->dev. That mcast is a temporary (used as an iterator) > variable that may be uninitialized. > There is no need to send the variable dev to the function, as each mcast > has its dev as a member in the mcast struct. Reviewed-by: Christoph Lameter -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] RXE: A Soft RoCE back-end for the RVT
This patch introduces an implementation of a back-end that works with RVT to make RoCE Verbs transport over any Ethernet network device. Example: After loading ib_rxe_net.ko echo eth1 > /sys/module/ib_rxe_net/parameters/add will create rvt0 IB device in RVT with Ethernet link layer --- drivers/infiniband/Kconfig|1 + drivers/infiniband/sw/Makefile|1 + drivers/infiniband/sw/rxe/Kconfig | 23 ++ drivers/infiniband/sw/rxe/Makefile|5 + drivers/infiniband/sw/rxe/rxe_net.c | 580 + drivers/infiniband/sw/rxe/rxe_net.h | 89 + drivers/infiniband/sw/rxe/rxe_sysfs.c | 167 ++ 7 files changed, 866 insertions(+), 0 deletions(-) 836 LOC SoftRoCE driver, impressive... +module_param_call(add, rxe_param_set_add, NULL, NULL, 0200); +module_param_call(remove, rxe_param_set_remove, NULL, NULL, 0200); The standard way of doing this is with class and device structures and not module_param_call... You can look at srp for example on how to get it right... -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 00/10] iSER support for remote invalidate
FYI, the only conflict as reported in linux-next last week is with nfsd tree and "IB: merge struct ib_device_attr into struct ib_device" here: http://marc.info/?l=linux-next&m=145155049101826&w=2 It looks like there is ongoing discussion this morning wrt rdma + nfsd trees.. Do you still want this series dropped from target-pending/for-next..? Given that the series was modified on top of the CQ API work then it makes better sense yes. Doug will take it. Thanks Nic. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv1 2/6] IB/core: Added members to support rdma cgroup
On Thu, Jan 07, 2016 at 04:46:19AM +0530, Parav Pandit wrote: > On Wed, Jan 6, 2016 at 3:26 AM, Tejun Heo wrote: > > On Wed, Jan 06, 2016 at 12:28:02AM +0530, Parav Pandit wrote: > >> Added function pointer table to store resource pool specific > >> operation for each resource type (verb and hw). > >> Added list node to link device to rdma cgroup so that it can > >> participate in resource accounting and limit configuration. > > > > Is there any point in splitting patches 1 and 2 from 3? > > > Patch 2 is in IB stack, so I separated that patch out from 1. That > makes it 3 patches. > If you think single patch is easier to review, let me know, I can > respin to have one patch for these 3 smaller patches. They don't do anything by themselves. I think putting them into a single patch would be easier to review. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv1 0/6] rdma controller support
Hello, Parav. On Thu, Jan 07, 2016 at 04:43:20AM +0530, Parav Pandit wrote: > > If different controllers can't agree upon the > > same set of resources, which probably is a pretty good sign that this > > isn't too well thought out to begin with, > > When you said "different controller" you meant "different hw vendors", right? > Or you meant, rdma, mem, cpu as controller here? Different hw vendors. > > at least make all resource > > types defined by the controller itself and let the controllers enable > > them selectively. > > > In this V1 patch, resource is defined by the IB stack and rdma cgroup > is facilitator for same. > By doing so, IB stack modules can define new resource without really > making changes to cgroup. > This design also allows hw vendors to define their own resources which > will be reviewed in rdma mailing list anway. > The idea is different hw versions can have different resource support, > so the whole intention is not about defining different resource but > rather enabling it. > But yes, I equally agree that by doing so, different hw controller > vendors can define different hw resources. How many vendors and resources are we talking about? What I was trying to say was that unless the number is extremely high, it'd be far simpler to hard code them in the rdma controller and let drivers enable the ones which apply to them. It would require updating the rdma cgroup controller to add new resource types but I think that'd actually be an upside, not down. There needs to be some checks and balances against adding new resource types; otherwise, it'll soon become a mess. Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/IPoIB: Fix kernel panic on multicast flow
On Thu, Jan 7, 2016 at 11:23 AM, Yuval Shaia wrote:
> On Thu, Jan 07, 2016 at 09:28:08AM +0200, Erez Shitrit wrote:
>>
>> ipoib_mcast_restart_task calls ipoib_mcast_remove_list with the
>> parameter mcast->dev. That mcast is a temporary (used as an iterator)
>> variable that may be uninitialized.
>> There is no need to send the variable dev to the function, as each mcast
> s/send/pass
>> has its dev as a member in the mcast struct.
>>
>> This causes the next panic:
>> RIP: 0010: ipoib_mcast_leave+0x6d/0xf0 [ib_ipoib]
>> RSP: 0018: EFLAGS: 00010246
>> RAX: f0201 RBX: 24e00 RCX: 0
>>
>>
>> Stack:
>> Call Trace:
>> ipoib_mcast_remove_list+0x3a/0x70 [ib_ipoib]
>> ipoib_mcast_restart_task+0x3bb/0x520 [ib_ipoib]
>> process_one_work+0x164/0x470
>> worker_thread+0x11d/0x420
>> ...
>>
>> Fixes: 5a0e81f6f483 ('IB/IPoIB: factor out common multicast list removal
>> code')
>> Signed-off-by: Erez Shitrit
>> Reported-by: Doron Tsur
>> ---
>> drivers/infiniband/ulp/ipoib/ipoib.h | 2 +-
>> drivers/infiniband/ulp/ipoib/ipoib_main.c | 3 +--
>> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 8
>> 3 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h
>> b/drivers/infiniband/ulp/ipoib/ipoib.h
>> index a924933..a6f3eab 100644
>> --- a/drivers/infiniband/ulp/ipoib/ipoib.h
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib.h
>> @@ -548,7 +548,7 @@ void ipoib_path_iter_read(struct ipoib_path_iter *iter,
>>
>> int ipoib_mcast_attach(struct net_device *dev, u16 mlid,
>> union ib_gid *mgid, int set_qkey);
>> -void ipoib_mcast_remove_list(struct net_device *dev, struct list_head
>> *remove_list);
>> +void ipoib_mcast_remove_list(struct list_head *remove_list);
>> void ipoib_check_and_add_mcast_sendonly(struct ipoib_dev_priv *priv, u8
>> *mgid,
>> struct list_head *remove_list);
>>
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> index 0a93cb2..25509bb 100644
>> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
>> @@ -1150,7 +1150,6 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv
>> *priv)
>> unsigned long flags;
>> int i;
>> LIST_HEAD(remove_list);
>> - struct net_device *dev = priv->dev;
> Isn't this one needed later? (in call to __ipoib_mcast_find made in commit
> bd99b2e05c)
There is a later commit (432c55ff) that already takes that code.
>>
>> if (test_bit(IPOIB_STOP_NEIGH_GC, &priv->flags))
>> return;
>> @@ -1196,7 +1195,7 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv
>> *priv)
>>
>> out_unlock:
>> spin_unlock_irqrestore(&priv->lock, flags);
>> - ipoib_mcast_remove_list(dev, &remove_list);
>> + ipoib_mcast_remove_list(&remove_list);
> I'm having difficulties applying this patch on linux-stable.git 4.4-rc6 and
> linux-next 4.4-rc8.
I sent it against ib-next
>> }
>>
>> static void ipoib_reap_neigh(struct work_struct *work)
>> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> index ab79b87..050dfa1 100644
>> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
>> @@ -723,12 +723,12 @@ void ipoib_check_and_add_mcast_sendonly(struct
>> ipoib_dev_priv *priv, u8 *mgid,
>> }
>> }
>>
>> -void ipoib_mcast_remove_list(struct net_device *dev, struct list_head
>> *remove_list)
>> +void ipoib_mcast_remove_list(struct list_head *remove_list)
>> {
>> struct ipoib_mcast *mcast, *tmcast;
>>
>> list_for_each_entry_safe(mcast, tmcast, remove_list, list) {
>> - ipoib_mcast_leave(dev, mcast);
>> + ipoib_mcast_leave(mcast->dev, mcast);
>> ipoib_mcast_free(mcast);
>> }
>> }
>> @@ -839,7 +839,7 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
>> if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
>> wait_for_completion(&mcast->done);
>>
>> - ipoib_mcast_remove_list(dev, &remove_list);
>> + ipoib_mcast_remove_list(&remove_list);
>> }
>>
>> static int ipoib_mcast_addr_is_valid(const u8 *addr, const u8 *broadcast)
>> @@ -965,7 +965,7 @@ void ipoib_mcast_restart_task(struct work_struct *work)
>> if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
>> wait_for_completion(&mcast->done);
>>
>> - ipoib_mcast_remove_list(mcast->dev, &remove_list);
>> + ipoib_mcast_remove_list(&remove_list);
>>
>> /*
>>* Double check that we are still up
>> --
>> 1.7.11.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line
Re: [PATCH] IB/IPoIB: Fix kernel panic on multicast flow
On Thu, Jan 07, 2016 at 09:28:08AM +0200, Erez Shitrit wrote:
>
> ipoib_mcast_restart_task calls ipoib_mcast_remove_list with the
> parameter mcast->dev. That mcast is a temporary (used as an iterator)
> variable that may be uninitialized.
> There is no need to send the variable dev to the function, as each mcast
s/send/pass
> has its dev as a member in the mcast struct.
>
> This causes the next panic:
> RIP: 0010: ipoib_mcast_leave+0x6d/0xf0 [ib_ipoib]
> RSP: 0018: EFLAGS: 00010246
> RAX: f0201 RBX: 24e00 RCX: 0
>
>
> Stack:
> Call Trace:
> ipoib_mcast_remove_list+0x3a/0x70 [ib_ipoib]
> ipoib_mcast_restart_task+0x3bb/0x520 [ib_ipoib]
> process_one_work+0x164/0x470
> worker_thread+0x11d/0x420
> ...
>
> Fixes: 5a0e81f6f483 ('IB/IPoIB: factor out common multicast list removal
> code')
> Signed-off-by: Erez Shitrit
> Reported-by: Doron Tsur
> ---
> drivers/infiniband/ulp/ipoib/ipoib.h | 2 +-
> drivers/infiniband/ulp/ipoib/ipoib_main.c | 3 +--
> drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 8
> 3 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib.h
> b/drivers/infiniband/ulp/ipoib/ipoib.h
> index a924933..a6f3eab 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib.h
> +++ b/drivers/infiniband/ulp/ipoib/ipoib.h
> @@ -548,7 +548,7 @@ void ipoib_path_iter_read(struct ipoib_path_iter *iter,
>
> int ipoib_mcast_attach(struct net_device *dev, u16 mlid,
> union ib_gid *mgid, int set_qkey);
> -void ipoib_mcast_remove_list(struct net_device *dev, struct list_head
> *remove_list);
> +void ipoib_mcast_remove_list(struct list_head *remove_list);
> void ipoib_check_and_add_mcast_sendonly(struct ipoib_dev_priv *priv, u8
> *mgid,
> struct list_head *remove_list);
>
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> index 0a93cb2..25509bb 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_main.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_main.c
> @@ -1150,7 +1150,6 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv
> *priv)
> unsigned long flags;
> int i;
> LIST_HEAD(remove_list);
> - struct net_device *dev = priv->dev;
Isn't this one needed later? (in call to __ipoib_mcast_find made in commit
bd99b2e05c)
>
> if (test_bit(IPOIB_STOP_NEIGH_GC, &priv->flags))
> return;
> @@ -1196,7 +1195,7 @@ static void __ipoib_reap_neigh(struct ipoib_dev_priv
> *priv)
>
> out_unlock:
> spin_unlock_irqrestore(&priv->lock, flags);
> - ipoib_mcast_remove_list(dev, &remove_list);
> + ipoib_mcast_remove_list(&remove_list);
I'm having difficulties applying this patch on linux-stable.git 4.4-rc6 and
linux-next 4.4-rc8.
> }
>
> static void ipoib_reap_neigh(struct work_struct *work)
> diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> index ab79b87..050dfa1 100644
> --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c
> @@ -723,12 +723,12 @@ void ipoib_check_and_add_mcast_sendonly(struct
> ipoib_dev_priv *priv, u8 *mgid,
> }
> }
>
> -void ipoib_mcast_remove_list(struct net_device *dev, struct list_head
> *remove_list)
> +void ipoib_mcast_remove_list(struct list_head *remove_list)
> {
> struct ipoib_mcast *mcast, *tmcast;
>
> list_for_each_entry_safe(mcast, tmcast, remove_list, list) {
> - ipoib_mcast_leave(dev, mcast);
> + ipoib_mcast_leave(mcast->dev, mcast);
> ipoib_mcast_free(mcast);
> }
> }
> @@ -839,7 +839,7 @@ void ipoib_mcast_dev_flush(struct net_device *dev)
> if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
> wait_for_completion(&mcast->done);
>
> - ipoib_mcast_remove_list(dev, &remove_list);
> + ipoib_mcast_remove_list(&remove_list);
> }
>
> static int ipoib_mcast_addr_is_valid(const u8 *addr, const u8 *broadcast)
> @@ -965,7 +965,7 @@ void ipoib_mcast_restart_task(struct work_struct *work)
> if (test_bit(IPOIB_MCAST_FLAG_BUSY, &mcast->flags))
> wait_for_completion(&mcast->done);
>
> - ipoib_mcast_remove_list(mcast->dev, &remove_list);
> + ipoib_mcast_remove_list(&remove_list);
>
> /*
>* Double check that we are still up
> --
> 1.7.11.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majord...@vger.kernel.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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
