* [PATCH AUTOSEL 4.14 08/27] IB/{qib, hfi1, rdmavt}: Correct ibv_devinfo max_mr value
From: Sasha Levin @ 2019-06-14 20:29 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Josh Collier, Dennis Dalessandro,
Jason Gunthorpe, Sasha Levin, linux-rdma
In-Reply-To: <20190614203018.27686-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 35164f5259a47ea756fa1deb3e463ac2a4f10dc9 ]
The command 'ibv_devinfo -v' reports 0 for max_mr.
Fix by assigning the query values after the mr lkey_table has been built
rather than early on in the driver.
Fixes: 7b1e2099adc8 ("IB/rdmavt: Move memory registration into rdmavt")
Reviewed-by: Josh Collier <josh.d.collier@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/verbs.c | 2 --
drivers/infiniband/hw/qib/qib_verbs.c | 2 --
drivers/infiniband/sw/rdmavt/mr.c | 2 ++
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index 2e8854ba18cf..f4372afa0e81 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1400,8 +1400,6 @@ static void hfi1_fill_device_attr(struct hfi1_devdata *dd)
rdi->dparms.props.max_cq = hfi1_max_cqs;
rdi->dparms.props.max_ah = hfi1_max_ahs;
rdi->dparms.props.max_cqe = hfi1_max_cqes;
- rdi->dparms.props.max_mr = rdi->lkey_table.max;
- rdi->dparms.props.max_fmr = rdi->lkey_table.max;
rdi->dparms.props.max_map_per_fmr = 32767;
rdi->dparms.props.max_pd = hfi1_max_pds;
rdi->dparms.props.max_qp_rd_atom = HFI1_MAX_RDMA_ATOMIC;
diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
index 9d92aeb8d9a1..350bc29a066f 100644
--- a/drivers/infiniband/hw/qib/qib_verbs.c
+++ b/drivers/infiniband/hw/qib/qib_verbs.c
@@ -1495,8 +1495,6 @@ static void qib_fill_device_attr(struct qib_devdata *dd)
rdi->dparms.props.max_cq = ib_qib_max_cqs;
rdi->dparms.props.max_cqe = ib_qib_max_cqes;
rdi->dparms.props.max_ah = ib_qib_max_ahs;
- rdi->dparms.props.max_mr = rdi->lkey_table.max;
- rdi->dparms.props.max_fmr = rdi->lkey_table.max;
rdi->dparms.props.max_map_per_fmr = 32767;
rdi->dparms.props.max_qp_rd_atom = QIB_MAX_RDMA_ATOMIC;
rdi->dparms.props.max_qp_init_rd_atom = 255;
diff --git a/drivers/infiniband/sw/rdmavt/mr.c b/drivers/infiniband/sw/rdmavt/mr.c
index e7013d2d4f0e..d5b51f4cb49a 100644
--- a/drivers/infiniband/sw/rdmavt/mr.c
+++ b/drivers/infiniband/sw/rdmavt/mr.c
@@ -96,6 +96,8 @@ int rvt_driver_mr_init(struct rvt_dev_info *rdi)
for (i = 0; i < rdi->lkey_table.max; i++)
RCU_INIT_POINTER(rdi->lkey_table.table[i], NULL);
+ rdi->dparms.props.max_mr = rdi->lkey_table.max;
+ rdi->dparms.props.max_fmr = rdi->lkey_table.max;
return 0;
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.14 09/27] IB/hfi1: Validate page aligned for a given virtual address
From: Sasha Levin @ 2019-06-14 20:29 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kamenee Arumugam, Michael J . Ruhl, Dennis Dalessandro,
Jason Gunthorpe, Sasha Levin, linux-rdma
In-Reply-To: <20190614203018.27686-1-sashal@kernel.org>
From: Kamenee Arumugam <kamenee.arumugam@intel.com>
[ Upstream commit 97736f36dbebf2cda2799db3b54717ba5b388255 ]
User applications can register memory regions for TID buffers that are not
aligned on page boundaries. Hfi1 is expected to pin those pages in memory
and cache the pages with mmu_rb. The rb tree will fail to insert pages
that are not aligned correctly.
Validate whether a given virtual address is page aligned before pinning.
Fixes: 7e7a436ecb6e ("staging/hfi1: Add TID entry program function body")
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Kamenee Arumugam <kamenee.arumugam@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/user_exp_rcv.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/infiniband/hw/hfi1/user_exp_rcv.c b/drivers/infiniband/hw/hfi1/user_exp_rcv.c
index 6f6c14df383e..b38e3808836c 100644
--- a/drivers/infiniband/hw/hfi1/user_exp_rcv.c
+++ b/drivers/infiniband/hw/hfi1/user_exp_rcv.c
@@ -324,6 +324,9 @@ int hfi1_user_exp_rcv_setup(struct hfi1_filedata *fd,
u32 *tidlist = NULL;
struct tid_user_buf *tidbuf;
+ if (!PAGE_ALIGNED(tinfo->vaddr))
+ return -EINVAL;
+
tidbuf = kzalloc(sizeof(*tidbuf), GFP_KERNEL);
if (!tidbuf)
return -ENOMEM;
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 04/18] IB/rdmavt: Fix alloc_qpn() WARN_ON()
From: Sasha Levin @ 2019-06-14 20:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Kaike Wan, Dennis Dalessandro, Jason Gunthorpe,
Sasha Levin, linux-rdma
In-Reply-To: <20190614203037.27910-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 2abae62a26a265129b364d8c1ef3be55e2c01309 ]
The qpn allocation logic has a WARN_ON() that intends to detect the use of
an index that will introduce bits in the lower order bits of the QOS bits
in the QPN.
Unfortunately, it has the following bugs:
- it misfires when wrapping QPN allocation for non-QOS
- it doesn't correctly detect low order QOS bits (despite the comment)
The WARN_ON() should not be applied to non-QOS (qos_shift == 1).
Additionally, it SHOULD test the qpn bits per the table below:
2 data VLs: [qp7, qp6, qp5, qp4, qp3, qp2, qp1] ^
[ 0, 0, 0, 0, 0, 0, sc0], qp bit 1 always 0*
3-4 data VLs: [qp7, qp6, qp5, qp4, qp3, qp2, qp1] ^
[ 0, 0, 0, 0, 0, sc1, sc0], qp bits [21] always 0
5-8 data VLs: [qp7, qp6, qp5, qp4, qp3, qp2, qp1] ^
[ 0, 0, 0, 0, sc2, sc1, sc0] qp bits [321] always 0
Fix by qualifying the warning for qos_shift > 1 and producing the correct
mask to insure the above bits are zero without generating a superfluous
warning.
Fixes: 501edc42446e ("IB/rdmavt: Correct warning during QPN allocation")
Reviewed-by: Kaike Wan <kaike.wan@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/sw/rdmavt/qp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c
index 6500c3b5a89c..8b330b53d636 100644
--- a/drivers/infiniband/sw/rdmavt/qp.c
+++ b/drivers/infiniband/sw/rdmavt/qp.c
@@ -370,7 +370,8 @@ static int alloc_qpn(struct rvt_dev_info *rdi, struct rvt_qpn_table *qpt,
offset = qpt->incr | ((offset & 1) ^ 1);
}
/* there can be no set bits in low-order QoS bits */
- WARN_ON(offset & (BIT(rdi->dparms.qos_shift) - 1));
+ WARN_ON(rdi->dparms.qos_shift > 1 &&
+ offset & ((BIT(rdi->dparms.qos_shift - 1) - 1) << 1));
qpn = mk_qpn(qpt, map, offset);
}
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 05/18] IB/hfi1: Insure freeze_work work_struct is canceled on shutdown
From: Sasha Levin @ 2019-06-14 20:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Michael J . Ruhl, Dennis Dalessandro,
Jason Gunthorpe, Sasha Levin, linux-rdma
In-Reply-To: <20190614203037.27910-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 6d517353c70bb0818b691ca003afdcb5ee5ea44e ]
By code inspection, the freeze_work is never canceled.
Fix by adding a cancel_work_sync in the shutdown path to insure it is no
longer running.
Fixes: 7724105686e7 ("IB/hfi1: add driver files")
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/chip.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c
index d30b3b908621..85db856047a9 100644
--- a/drivers/infiniband/hw/hfi1/chip.c
+++ b/drivers/infiniband/hw/hfi1/chip.c
@@ -9620,6 +9620,7 @@ void hfi1_quiet_serdes(struct hfi1_pportdata *ppd)
/* disable the port */
clear_rcvctrl(dd, RCV_CTRL_RCV_PORT_ENABLE_SMASK);
+ cancel_work_sync(&ppd->freeze_work);
}
static inline int init_cpu_counters(struct hfi1_devdata *dd)
--
2.20.1
^ permalink raw reply related
* [PATCH AUTOSEL 4.9 06/18] IB/{qib, hfi1, rdmavt}: Correct ibv_devinfo max_mr value
From: Sasha Levin @ 2019-06-14 20:30 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mike Marciniszyn, Josh Collier, Dennis Dalessandro,
Jason Gunthorpe, Sasha Levin, linux-rdma
In-Reply-To: <20190614203037.27910-1-sashal@kernel.org>
From: Mike Marciniszyn <mike.marciniszyn@intel.com>
[ Upstream commit 35164f5259a47ea756fa1deb3e463ac2a4f10dc9 ]
The command 'ibv_devinfo -v' reports 0 for max_mr.
Fix by assigning the query values after the mr lkey_table has been built
rather than early on in the driver.
Fixes: 7b1e2099adc8 ("IB/rdmavt: Move memory registration into rdmavt")
Reviewed-by: Josh Collier <josh.d.collier@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/infiniband/hw/hfi1/verbs.c | 2 --
drivers/infiniband/hw/qib/qib_verbs.c | 2 --
drivers/infiniband/sw/rdmavt/mr.c | 2 ++
3 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index d9c71750e22d..15054a0cbf6d 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1344,8 +1344,6 @@ static void hfi1_fill_device_attr(struct hfi1_devdata *dd)
rdi->dparms.props.max_cq = hfi1_max_cqs;
rdi->dparms.props.max_ah = hfi1_max_ahs;
rdi->dparms.props.max_cqe = hfi1_max_cqes;
- rdi->dparms.props.max_mr = rdi->lkey_table.max;
- rdi->dparms.props.max_fmr = rdi->lkey_table.max;
rdi->dparms.props.max_map_per_fmr = 32767;
rdi->dparms.props.max_pd = hfi1_max_pds;
rdi->dparms.props.max_qp_rd_atom = HFI1_MAX_RDMA_ATOMIC;
diff --git a/drivers/infiniband/hw/qib/qib_verbs.c b/drivers/infiniband/hw/qib/qib_verbs.c
index 954f15064514..d6e183775e24 100644
--- a/drivers/infiniband/hw/qib/qib_verbs.c
+++ b/drivers/infiniband/hw/qib/qib_verbs.c
@@ -1568,8 +1568,6 @@ static void qib_fill_device_attr(struct qib_devdata *dd)
rdi->dparms.props.max_cq = ib_qib_max_cqs;
rdi->dparms.props.max_cqe = ib_qib_max_cqes;
rdi->dparms.props.max_ah = ib_qib_max_ahs;
- rdi->dparms.props.max_mr = rdi->lkey_table.max;
- rdi->dparms.props.max_fmr = rdi->lkey_table.max;
rdi->dparms.props.max_map_per_fmr = 32767;
rdi->dparms.props.max_qp_rd_atom = QIB_MAX_RDMA_ATOMIC;
rdi->dparms.props.max_qp_init_rd_atom = 255;
diff --git a/drivers/infiniband/sw/rdmavt/mr.c b/drivers/infiniband/sw/rdmavt/mr.c
index 49d55a0322f6..dbd4c0d268e9 100644
--- a/drivers/infiniband/sw/rdmavt/mr.c
+++ b/drivers/infiniband/sw/rdmavt/mr.c
@@ -94,6 +94,8 @@ int rvt_driver_mr_init(struct rvt_dev_info *rdi)
for (i = 0; i < rdi->lkey_table.max; i++)
RCU_INIT_POINTER(rdi->lkey_table.table[i], NULL);
+ rdi->dparms.props.max_mr = rdi->lkey_table.max;
+ rdi->dparms.props.max_fmr = rdi->lkey_table.max;
return 0;
}
--
2.20.1
^ permalink raw reply related
* Re: RDMA: Clean destroy CQ in drivers do not return errors
From: Leon Romanovsky @ 2019-06-15 7:12 UTC (permalink / raw)
To: Doug Ledford
Cc: Colin Ian King, Gal Pressman, Dennis Dalessandro,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ee0c017e93e28317791b7395e257801a208c7306.camel@redhat.com>
On Fri, Jun 14, 2019 at 03:46:50PM -0400, Doug Ledford wrote:
> On Fri, 2019-06-14 at 14:59 +0100, Colin Ian King wrote:
> > Hi,
> >
> > Static analysis with Coverity reported an issue with the following
> > commit:
> >
> > commit a52c8e2469c30cf7ac453d624aed9c168b23d1af
> > Author: Leon Romanovsky <leonro@mellanox.com>
> > Date: Tue May 28 14:37:28 2019 +0300
> >
> > RDMA: Clean destroy CQ in drivers do not return errors
> >
> > In function bnxt_re_destroy_cq() contains the following:
> >
> > if (!cq->umem)
> > ib_umem_release(cq->umem);
>
> Given that the original test that was replaced was:
> if (!IS_ERR_OR_NULL(cq->umem))
>
> we aren't really worried about a null cq, just that umem is valid. So,
> the logic is inverted on the test (or possibly we shouldn't have
> replaced !IS_ERR_OR_NULL(cq->umem) at all).
I took a very brief look and think that the better way will be to put
this "if (null)" check inside ib_umem_release() and make unconditional
call to that function in all call sites.
>
> But on closer inspection, the bnxt_re specific portion of this patch
> appears to have another problem in that it no longer checks the result
> of bnxt_qplib_destroy_cq() yet it does nothing to keep that function
> from failing.
It was intentional for two reasons. First, bnxt_re already had exactly
same logic without any checks of returned call inside bnxt_re_create_cq().
Second, we need to release kernel memory without any relation to HW state.
Maybe I should move bnxt_qplib_free_hwq() to be immediately after
bnxt_qplib_rcfw_send_message() inside of bnxt_qplib_destroy_cq()?
>
> Leon, can you send a followup fix?
Sure, I'll do it tomorrow.
>
> > Coverity detects this as a deference after null check on the null
> > pointer cq->umem:
> >
> > "var_deref_model: Passing null pointer cq->umem to ib_umem_release,
> > which dereferences it"
> >
> > Is the logic inverted on that null check?
> >
> > Colin
>
> --
> Doug Ledford <dledford@redhat.com>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
> 2FDD
^ permalink raw reply
* Re: [PATCH 2/2] ipoib: show VF broadcast address
From: Denis Kirjanov @ 2019-06-15 8:49 UTC (permalink / raw)
To: Doug Ledford; +Cc: Denis Kirjanov, davem, netdev, linux-rdma, mkubecek
In-Reply-To: <f91615fe4a883ccb6490aec11ef4ae64cdd9e494.camel@redhat.com>
On 6/14/19, Doug Ledford <dledford@redhat.com> wrote:
> On Fri, 2019-06-14 at 15:32 +0200, Denis Kirjanov wrote:
>> in IPoIB case we can't see a VF broadcast address for but
>> can see for PF
>>
>> Before:
>> 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
>> state UP mode DEFAULT group default qlen 256
>> link/infiniband
>> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
>> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
>> vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state
>> disable,
>> trust off, query_rss off
>> ...
>
> The above Before: output should be used as the After: portion of the
> previous commit message. The previos commit does not fully resolve the
> problem, but yet the commit message acts as though it does.
>
>>
>> After:
>> 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
>> state UP mode DEFAULT group default qlen 256
>> link/infiniband
>> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
>> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
>> vf 0 link/infiniband
>> 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
>> 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff, spoof
>> checking off, link-state disable, trust off, query_rss off
>
> Ok, I get why the After: should have a valid broadcast. What I don't
> get is why the Before: shows a MAC and the After: shows a
> link/infiniband? What change in this patch is responsible for that
> difference? I honestly expect, by reading this patch, that you would
> have a MAC and Broadcast that look like Ethernet, not that the full
> issue would be resolved.
Hi Doug,
it's the patch for iproute2 that I'm going to send
>
>> v1->v2: add the IFLA_VF_BROADCAST constant
>> v2->v3: put IFLA_VF_BROADCAST at the end
>> to avoid KABI breakage and set NLA_REJECT
>> dev_setlink
>>
>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
>> ---
>> include/uapi/linux/if_link.h | 5 +++++
>> net/core/rtnetlink.c | 5 +++++
>> 2 files changed, 10 insertions(+)
>>
>> diff --git a/include/uapi/linux/if_link.h
>> b/include/uapi/linux/if_link.h
>> index 5b225ff63b48..6f75bda2c2d7 100644
>> --- a/include/uapi/linux/if_link.h
>> +++ b/include/uapi/linux/if_link.h
>> @@ -694,6 +694,7 @@ enum {
>> IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
>> IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
>> IFLA_VF_VLAN_LIST, /* nested list of vlans, option for
>> QinQ */
>> + IFLA_VF_BROADCAST, /* VF broadcast */
>> __IFLA_VF_MAX,
>> };
>>
>> @@ -704,6 +705,10 @@ struct ifla_vf_mac {
>> __u8 mac[32]; /* MAX_ADDR_LEN */
>> };
>>
>> +struct ifla_vf_broadcast {
>> + __u8 broadcast[32];
>> +};
>> +
>> struct ifla_vf_vlan {
>> __u32 vf;
>> __u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index cec60583931f..8ac81630ab5c 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -908,6 +908,7 @@ static inline int rtnl_vfinfo_size(const struct
>> net_device *dev,
>> size += num_vfs *
>> (nla_total_size(0) +
>> nla_total_size(sizeof(struct ifla_vf_mac)) +
>> + nla_total_size(sizeof(struct
>> ifla_vf_broadcast)) +
>> nla_total_size(sizeof(struct ifla_vf_vlan)) +
>> nla_total_size(0) + /* nest IFLA_VF_VLAN_LIST
>> */
>> nla_total_size(MAX_VLAN_LIST_LEN *
>> @@ -1197,6 +1198,7 @@ static noinline_for_stack int
>> rtnl_fill_vfinfo(struct sk_buff *skb,
>> struct ifla_vf_vlan vf_vlan;
>> struct ifla_vf_rate vf_rate;
>> struct ifla_vf_mac vf_mac;
>> + struct ifla_vf_broadcast vf_broadcast;
>> struct ifla_vf_info ivi;
>>
>> memset(&ivi, 0, sizeof(ivi));
>> @@ -1231,6 +1233,7 @@ static noinline_for_stack int
>> rtnl_fill_vfinfo(struct sk_buff *skb,
>> vf_trust.vf = ivi.vf;
>>
>> memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
>> + memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
>> vf_vlan.vlan = ivi.vlan;
>> vf_vlan.qos = ivi.qos;
>> vf_vlan_info.vlan = ivi.vlan;
>> @@ -1247,6 +1250,7 @@ static noinline_for_stack int
>> rtnl_fill_vfinfo(struct sk_buff *skb,
>> if (!vf)
>> goto nla_put_vfinfo_failure;
>> if (nla_put(skb, IFLA_VF_MAC, sizeof(vf_mac), &vf_mac) ||
>> + nla_put(skb, IFLA_VF_BROADCAST, sizeof(vf_broadcast),
>> &vf_broadcast) ||
>> nla_put(skb, IFLA_VF_VLAN, sizeof(vf_vlan), &vf_vlan) ||
>> nla_put(skb, IFLA_VF_RATE, sizeof(vf_rate),
>> &vf_rate) ||
>> @@ -1753,6 +1757,7 @@ static const struct nla_policy
>> ifla_info_policy[IFLA_INFO_MAX+1] = {
>>
>> static const struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
>> [IFLA_VF_MAC] = { .len = sizeof(struct ifla_vf_mac)
>> },
>> + [IFLA_VF_BROADCAST] = { .type = NLA_REJECT },
>> [IFLA_VF_VLAN] = { .len = sizeof(struct
>> ifla_vf_vlan) },
>> [IFLA_VF_VLAN_LIST] = { .type = NLA_NESTED },
>> [IFLA_VF_TX_RATE] = { .len = sizeof(struct ifla_vf_tx_rate) },
>
> --
> Doug Ledford <dledford@redhat.com>
> GPG KeyID: B826A3330E572FDD
> Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
> 2FDD
>
--
Regards / Mit besten Grüßen,
Denis
^ permalink raw reply
* [PATCH] ipaddress: correctly print a VF hw address in the IPoIB case
From: Denis Kirjanov @ 2019-06-15 11:40 UTC (permalink / raw)
To: stephen; +Cc: dledford, netdev, linux-rdma, mkubecek, Denis Kirjanov
Current code assumes that we print Etheret mac and
that doesn't work in IPoIB case with SRIOV-enabled hardware
Before:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state disable,
trust off, query_rss off
...
After:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff, spoof
checking off, link-state disable, trust off, query_rss off
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
include/uapi/linux/if_infiniband.h | 29 +++++++++++++++++++++++++++
include/uapi/linux/if_link.h | 5 +++++
include/uapi/linux/netdevice.h | 2 +-
ip/ipaddress.c | 41 +++++++++++++++++++++++++++++++++-----
4 files changed, 71 insertions(+), 6 deletions(-)
create mode 100644 include/uapi/linux/if_infiniband.h
diff --git a/include/uapi/linux/if_infiniband.h b/include/uapi/linux/if_infiniband.h
new file mode 100644
index 00000000..7d958475
--- /dev/null
+++ b/include/uapi/linux/if_infiniband.h
@@ -0,0 +1,29 @@
+/*
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available at
+ * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
+ * license, available in the LICENSE.TXT file accompanying this
+ * software. These details are also available at
+ * <http://www.openfabrics.org/software_license.htm>.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Copyright (c) 2004 Topspin Communications. All rights reserved.
+ *
+ * $Id$
+ */
+
+#ifndef _LINUX_IF_INFINIBAND_H
+#define _LINUX_IF_INFINIBAND_H
+
+#define INFINIBAND_ALEN 20 /* Octets in IPoIB HW addr */
+
+#endif /* _LINUX_IF_INFINIBAND_H */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index bfe7f9e6..831f1849 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -692,6 +692,7 @@ enum {
IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
IFLA_VF_VLAN_LIST, /* nested list of vlans, option for QinQ */
+ IFLA_VF_BROADCAST, /* VF broadcast */
__IFLA_VF_MAX,
};
@@ -702,6 +703,10 @@ struct ifla_vf_mac {
__u8 mac[32]; /* MAX_ADDR_LEN */
};
+struct ifla_vf_broadcast {
+ __u8 broadcast[32];
+};
+
struct ifla_vf_vlan {
__u32 vf;
__u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
diff --git a/include/uapi/linux/netdevice.h b/include/uapi/linux/netdevice.h
index 86d961c9..aaa48818 100644
--- a/include/uapi/linux/netdevice.h
+++ b/include/uapi/linux/netdevice.h
@@ -30,7 +30,7 @@
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/if_link.h>
-
+#include <linux/if_infiniband.h>
#define MAX_ADDR_LEN 32 /* Largest hardware address length */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b504200b..99e62621 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -349,9 +349,10 @@ static void print_af_spec(FILE *fp, struct rtattr *af_spec_attr)
static void print_vf_stats64(FILE *fp, struct rtattr *vfstats);
-static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
+static void print_vfinfo(struct ifinfomsg *ifi, FILE *fp, struct rtattr *vfinfo)
{
struct ifla_vf_mac *vf_mac;
+ struct ifla_vf_broadcast *vf_broadcast;
struct ifla_vf_tx_rate *vf_tx_rate;
struct rtattr *vf[IFLA_VF_MAX + 1] = {};
@@ -365,13 +366,43 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
parse_rtattr_nested(vf, IFLA_VF_MAX, vfinfo);
vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
+ vf_broadcast = RTA_DATA(vf[IFLA_VF_BROADCAST]);
vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
print_string(PRINT_FP, NULL, "%s ", _SL_);
print_int(PRINT_ANY, "vf", "vf %d ", vf_mac->vf);
- print_string(PRINT_ANY, "mac", "MAC %s",
- ll_addr_n2a((unsigned char *) &vf_mac->mac,
- ETH_ALEN, 0, b1, sizeof(b1)));
+
+ print_string(PRINT_ANY,
+ "link_type",
+ " link/%s ",
+ ll_type_n2a(ifi->ifi_type, b1, sizeof(b1)));
+
+ print_color_string(PRINT_ANY,
+ COLOR_MAC,
+ "address",
+ "%s",
+ ll_addr_n2a((unsigned char *) &vf_mac->mac,
+ ifi->ifi_type == ARPHRD_ETHER ? ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+
+ if (vf[IFLA_VF_BROADCAST]) {
+ if (ifi->ifi_flags&IFF_POINTOPOINT) {
+ print_string(PRINT_FP, NULL, " peer ", NULL);
+ print_bool(PRINT_JSON,
+ "link_pointtopoint", NULL, true);
+ } else {
+ print_string(PRINT_FP, NULL, " brd ", NULL);
+ }
+ print_color_string(PRINT_ANY,
+ COLOR_MAC,
+ "broadcast",
+ "%s",
+ ll_addr_n2a((unsigned char *) &vf_broadcast->broadcast,
+ ifi->ifi_type == ARPHRD_ETHER ? ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+ }
if (vf[IFLA_VF_VLAN_LIST]) {
struct rtattr *i, *vfvlanlist = vf[IFLA_VF_VLAN_LIST];
@@ -1102,7 +1133,7 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
open_json_array(PRINT_JSON, "vfinfo_list");
for (i = RTA_DATA(vflist); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
open_json_object(NULL);
- print_vfinfo(fp, i);
+ print_vfinfo(ifi, fp, i);
close_json_object();
}
close_json_array(PRINT_JSON, NULL);
--
2.12.3
^ permalink raw reply related
* [iproute2] ipaddress: correctly print a VF hw address in the IPoIB case
From: Denis Kirjanov @ 2019-06-15 11:40 UTC (permalink / raw)
To: stephen; +Cc: dledford, netdev, linux-rdma, mkubecek, Denis Kirjanov
In-Reply-To: <20190615114056.100808-1-dkirjanov@suse.com>
Current code assumes that we print Etheret mac and
that doesn't work in IPoIB case with SRIOV-enabled hardware
Before:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state disable,
trust off, query_rss off
...
After:
11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc pfifo_fast
state UP mode DEFAULT group default qlen 256
link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
vf 0 link/infiniband
80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff, spoof
checking off, link-state disable, trust off, query_rss off
Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
include/uapi/linux/if_infiniband.h | 29 +++++++++++++++++++++++++++
include/uapi/linux/if_link.h | 5 +++++
include/uapi/linux/netdevice.h | 2 +-
ip/ipaddress.c | 41 +++++++++++++++++++++++++++++++++-----
4 files changed, 71 insertions(+), 6 deletions(-)
create mode 100644 include/uapi/linux/if_infiniband.h
diff --git a/include/uapi/linux/if_infiniband.h b/include/uapi/linux/if_infiniband.h
new file mode 100644
index 00000000..7d958475
--- /dev/null
+++ b/include/uapi/linux/if_infiniband.h
@@ -0,0 +1,29 @@
+/*
+ * This software is available to you under a choice of one of two
+ * licenses. You may choose to be licensed under the terms of the GNU
+ * General Public License (GPL) Version 2, available at
+ * <http://www.fsf.org/copyleft/gpl.html>, or the OpenIB.org BSD
+ * license, available in the LICENSE.TXT file accompanying this
+ * software. These details are also available at
+ * <http://www.openfabrics.org/software_license.htm>.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Copyright (c) 2004 Topspin Communications. All rights reserved.
+ *
+ * $Id$
+ */
+
+#ifndef _LINUX_IF_INFINIBAND_H
+#define _LINUX_IF_INFINIBAND_H
+
+#define INFINIBAND_ALEN 20 /* Octets in IPoIB HW addr */
+
+#endif /* _LINUX_IF_INFINIBAND_H */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index bfe7f9e6..831f1849 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -692,6 +692,7 @@ enum {
IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
IFLA_VF_VLAN_LIST, /* nested list of vlans, option for QinQ */
+ IFLA_VF_BROADCAST, /* VF broadcast */
__IFLA_VF_MAX,
};
@@ -702,6 +703,10 @@ struct ifla_vf_mac {
__u8 mac[32]; /* MAX_ADDR_LEN */
};
+struct ifla_vf_broadcast {
+ __u8 broadcast[32];
+};
+
struct ifla_vf_vlan {
__u32 vf;
__u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
diff --git a/include/uapi/linux/netdevice.h b/include/uapi/linux/netdevice.h
index 86d961c9..aaa48818 100644
--- a/include/uapi/linux/netdevice.h
+++ b/include/uapi/linux/netdevice.h
@@ -30,7 +30,7 @@
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/if_link.h>
-
+#include <linux/if_infiniband.h>
#define MAX_ADDR_LEN 32 /* Largest hardware address length */
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index b504200b..99e62621 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -349,9 +349,10 @@ static void print_af_spec(FILE *fp, struct rtattr *af_spec_attr)
static void print_vf_stats64(FILE *fp, struct rtattr *vfstats);
-static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
+static void print_vfinfo(struct ifinfomsg *ifi, FILE *fp, struct rtattr *vfinfo)
{
struct ifla_vf_mac *vf_mac;
+ struct ifla_vf_broadcast *vf_broadcast;
struct ifla_vf_tx_rate *vf_tx_rate;
struct rtattr *vf[IFLA_VF_MAX + 1] = {};
@@ -365,13 +366,43 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
parse_rtattr_nested(vf, IFLA_VF_MAX, vfinfo);
vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
+ vf_broadcast = RTA_DATA(vf[IFLA_VF_BROADCAST]);
vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
print_string(PRINT_FP, NULL, "%s ", _SL_);
print_int(PRINT_ANY, "vf", "vf %d ", vf_mac->vf);
- print_string(PRINT_ANY, "mac", "MAC %s",
- ll_addr_n2a((unsigned char *) &vf_mac->mac,
- ETH_ALEN, 0, b1, sizeof(b1)));
+
+ print_string(PRINT_ANY,
+ "link_type",
+ " link/%s ",
+ ll_type_n2a(ifi->ifi_type, b1, sizeof(b1)));
+
+ print_color_string(PRINT_ANY,
+ COLOR_MAC,
+ "address",
+ "%s",
+ ll_addr_n2a((unsigned char *) &vf_mac->mac,
+ ifi->ifi_type == ARPHRD_ETHER ? ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+
+ if (vf[IFLA_VF_BROADCAST]) {
+ if (ifi->ifi_flags&IFF_POINTOPOINT) {
+ print_string(PRINT_FP, NULL, " peer ", NULL);
+ print_bool(PRINT_JSON,
+ "link_pointtopoint", NULL, true);
+ } else {
+ print_string(PRINT_FP, NULL, " brd ", NULL);
+ }
+ print_color_string(PRINT_ANY,
+ COLOR_MAC,
+ "broadcast",
+ "%s",
+ ll_addr_n2a((unsigned char *) &vf_broadcast->broadcast,
+ ifi->ifi_type == ARPHRD_ETHER ? ETH_ALEN : INFINIBAND_ALEN,
+ ifi->ifi_type,
+ b1, sizeof(b1)));
+ }
if (vf[IFLA_VF_VLAN_LIST]) {
struct rtattr *i, *vfvlanlist = vf[IFLA_VF_VLAN_LIST];
@@ -1102,7 +1133,7 @@ int print_linkinfo(struct nlmsghdr *n, void *arg)
open_json_array(PRINT_JSON, "vfinfo_list");
for (i = RTA_DATA(vflist); RTA_OK(i, rem); i = RTA_NEXT(i, rem)) {
open_json_object(NULL);
- print_vfinfo(fp, i);
+ print_vfinfo(ifi, fp, i);
close_json_object();
}
close_json_array(PRINT_JSON, NULL);
--
2.12.3
^ permalink raw reply related
* Re: [PATCH v3 hmm 01/12] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Christoph Hellwig @ 2019-06-15 13:56 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-2-jgg-uk2M96/98Pc@public.gmane.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 02/12] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Christoph Hellwig @ 2019-06-15 13:59 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-3-jgg-uk2M96/98Pc@public.gmane.org>
On Thu, Jun 13, 2019 at 09:44:40PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> Ralph observes that hmm_range_register() can only be called by a driver
> while a mirror is registered. Make this clear in the API by passing in the
> mirror structure as a parameter.
>
> This also simplifies understanding the lifetime model for struct hmm, as
> the hmm pointer must be valid as part of a registered mirror so all we
> need in hmm_register_range() is a simple kref_get.
Looks good, at least an an intermediate step:
Reviewed-by: Christoph Hellwig <hch@lst.de>
> index f6956d78e3cb25..22a97ada108b4e 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -914,13 +914,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
> * Track updates to the CPU page table see include/linux/hmm.h
> */
> int hmm_range_register(struct hmm_range *range,
> - struct mm_struct *mm,
> + struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift)
> {
> unsigned long mask = ((1UL << page_shift) - 1UL);
> - struct hmm *hmm;
> + struct hmm *hmm = mirror->hmm;
>
> range->valid = false;
> range->hmm = NULL;
> @@ -934,20 +934,15 @@ int hmm_range_register(struct hmm_range *range,
> range->start = start;
> range->end = end;
But while you're at it: the calling conventions of hmm_range_register
are still rather odd, as the staet, end and page_shift arguments are
only used to fill out fields in the range structure passed in. Might
be worth cleaning up as well if we change the calling convention.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 03/12] mm/hmm: Hold a mmgrab from hmm to mm
From: Christoph Hellwig @ 2019-06-15 13:59 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-4-jgg-uk2M96/98Pc@public.gmane.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 04/12] mm/hmm: Simplify hmm_get_or_create and make it reliable
From: Christoph Hellwig @ 2019-06-15 14:12 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-5-jgg-uk2M96/98Pc@public.gmane.org>
> + spin_lock(&mm->page_table_lock);
> + if (mm->hmm) {
> + if (kref_get_unless_zero(&mm->hmm->kref)) {
> + spin_unlock(&mm->page_table_lock);
> + return mm->hmm;
> + }
> + }
> + spin_unlock(&mm->page_table_lock);
This could become:
spin_lock(&mm->page_table_lock);
hmm = mm->hmm
if (hmm && kref_get_unless_zero(&hmm->kref))
goto out_unlock;
spin_unlock(&mm->page_table_lock);
as the last two lines of the function already drop the page_table_lock
and then return hmm. Or drop the "hmm = mm->hmm" asignment above and
return mm->hmm as that should be always identical to hmm at the end
to save another line.
> + /*
> + * The mm->hmm pointer is kept valid while notifier ops can be running
> + * so they don't have to deal with a NULL mm->hmm value
> + */
The comment confuses me. How does the page_table_lock relate to
possibly running notifiers, as I can't find that we take
page_table_lock? Or is it just about the fact that we only clear
mm->hmm in the free callback, and not in hmm_free?
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 05/12] mm/hmm: Remove duplicate condition test before wait_event_timeout
From: Christoph Hellwig @ 2019-06-15 14:12 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-6-jgg-uk2M96/98Pc@public.gmane.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 06/12] mm/hmm: Hold on to the mmget for the lifetime of the range
From: Christoph Hellwig @ 2019-06-15 14:14 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-7-jgg-uk2M96/98Pc@public.gmane.org>
> mutex_lock(&hmm->lock);
> - list_for_each_entry(range, &hmm->ranges, list)
> - range->valid = false;
> - wake_up_all(&hmm->wq);
> + /*
> + * Since hmm_range_register() holds the mmget() lock hmm_release() is
> + * prevented as long as a range exists.
> + */
> + WARN_ON(!list_empty(&hmm->ranges));
> mutex_unlock(&hmm->lock);
This can just use list_empty_careful and avoid the lock entirely.
Otherwise looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 07/12] mm/hmm: Use lockdep instead of comments
From: Christoph Hellwig @ 2019-06-15 14:14 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Souptick Joarder,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-8-jgg-uk2M96/98Pc@public.gmane.org>
Looks good,
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 08/12] mm/hmm: Remove racy protection against double-unregistration
From: Christoph Hellwig @ 2019-06-15 14:16 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-9-jgg-uk2M96/98Pc@public.gmane.org>
On Thu, Jun 13, 2019 at 09:44:46PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> No other register/unregister kernel API attempts to provide this kind of
> protection as it is inherently racy, so just drop it.
>
> Callers should provide their own protection, it appears nouveau already
> does, but just in case drop a debugging POISON.
I don't even think we even need to bother with the POISON, normal list
debugging will already catch a double unregistration anyway.
Otherwise looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 09/12] mm/hmm: Poison hmm_range during unregister
From: Christoph Hellwig @ 2019-06-15 14:17 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Souptick Joarder,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ira Weiny, Ben Skeggs
In-Reply-To: <20190614004450.20252-10-jgg-uk2M96/98Pc@public.gmane.org>
> - /* Sanity check this really should not happen. */
> - if (hmm == NULL || range->end <= range->start)
> - return;
> -
> mutex_lock(&hmm->lock);
> list_del_rcu(&range->list);
> mutex_unlock(&hmm->lock);
>
> /* Drop reference taken by hmm_range_register() */
> - range->valid = false;
> mmput(hmm->mm);
> hmm_put(hmm);
> - range->hmm = NULL;
> +
> + /*
> + * The range is now invalid and the ref on the hmm is dropped, so
> + * poison the pointer. Leave other fields in place, for the caller's
> + * use.
> + */
> + range->valid = false;
> + memset(&range->hmm, POISON_INUSE, sizeof(range->hmm));
Formatting seems to be messed up. But again I don't see the value
in the poisoning, just let normal linked list debugging do its work.
The other cleanups looks fine to me.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 10/12] mm/hmm: Do not use list*_rcu() for hmm->ranges
From: Christoph Hellwig @ 2019-06-15 14:18 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo, Ira Weiny,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Souptick Joarder,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-11-jgg-uk2M96/98Pc@public.gmane.org>
On Thu, Jun 13, 2019 at 09:44:48PM -0300, Jason Gunthorpe wrote:
> range->hmm = hmm;
> kref_get(&hmm->kref);
> - list_add_rcu(&range->list, &hmm->ranges);
> + list_add(&range->list, &hmm->ranges);
>
> /*
> * If there are any concurrent notifiers we have to wait for them for
> @@ -934,7 +934,7 @@ void hmm_range_unregister(struct hmm_range *range)
> struct hmm *hmm = range->hmm;
>
> mutex_lock(&hmm->lock);
> - list_del_rcu(&range->list);
> + list_del(&range->list);
> mutex_unlock(&hmm->lock);
Looks fine:
Signed-off-by: Christoph Hellwig <hch@lst.de>
Btw, is there any reason new ranges are added to the front and not the
tail of the list?
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 11/12] mm/hmm: Remove confusing comment and logic from hmm_release
From: Christoph Hellwig @ 2019-06-15 14:21 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-12-jgg-uk2M96/98Pc@public.gmane.org>
On Thu, Jun 13, 2019 at 09:44:49PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> hmm_release() is called exactly once per hmm. ops->release() cannot
> accidentally trigger any action that would recurse back onto
> hmm->mirrors_sem.
In linux-next amdgpu actually calls hmm_mirror_unregister from its
release function. That whole release function looks rather sketchy,
but we probably need to sort that out first.
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH v3 hmm 12/12] mm/hmm: Fix error flows in hmm_invalidate_range_start
From: Christoph Hellwig @ 2019-06-15 14:25 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Andrea Arcangeli, Philip Yang, Ralph Campbell,
linux-rdma-u79uwXL29TY76Z2rM5mHXA, John Hubbard,
Felix.Kuehling-5C7GfCeVMHo,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg, Jerome Glisse, Jason Gunthorpe,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs
In-Reply-To: <20190614004450.20252-13-jgg-uk2M96/98Pc@public.gmane.org>
On Thu, Jun 13, 2019 at 09:44:50PM -0300, Jason Gunthorpe wrote:
> From: Jason Gunthorpe <jgg@mellanox.com>
>
> If the trylock on the hmm->mirrors_sem fails the function will return
> without decrementing the notifiers that were previously incremented. Since
> the caller will not call invalidate_range_end() on EAGAIN this will result
> in notifiers becoming permanently incremented and deadlock.
>
> If the sync_cpu_device_pagetables() required blocking the function will
> not return EAGAIN even though the device continues to touch the
> pages. This is a violation of the mmu notifier contract.
>
> Switch, and rename, the ranges_lock to a spin lock so we can reliably
> obtain it without blocking during error unwind.
>
> The error unwind is necessary since the notifiers count must be held
> incremented across the call to sync_cpu_device_pagetables() as we cannot
> allow the range to become marked valid by a parallel
> invalidate_start/end() pair while doing sync_cpu_device_pagetables().
>
> Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
> Reviewed-by: Ralph Campbell <rcampbell@nvidia.com>
> Tested-by: Philip Yang <Philip.Yang@amd.com>
> ---
> include/linux/hmm.h | 2 +-
> mm/hmm.c | 77 +++++++++++++++++++++++++++------------------
> 2 files changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index bf013e96525771..0fa8ea34ccef6d 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -86,7 +86,7 @@
> struct hmm {
> struct mm_struct *mm;
> struct kref kref;
> - struct mutex lock;
> + spinlock_t ranges_lock;
> struct list_head ranges;
> struct list_head mirrors;
> struct mmu_notifier mmu_notifier;
> diff --git a/mm/hmm.c b/mm/hmm.c
> index c0d43302fd6b2f..1172a4f0206963 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -67,7 +67,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> init_rwsem(&hmm->mirrors_sem);
> hmm->mmu_notifier.ops = NULL;
> INIT_LIST_HEAD(&hmm->ranges);
> - mutex_init(&hmm->lock);
> + spin_lock_init(&hmm->ranges_lock);
> kref_init(&hmm->kref);
> hmm->notifiers = 0;
> hmm->mm = mm;
> @@ -124,18 +124,19 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> + unsigned long flags;
>
> /* Bail out if hmm is in the process of being freed */
> if (!kref_get_unless_zero(&hmm->kref))
> return;
>
> - mutex_lock(&hmm->lock);
> + spin_lock_irqsave(&hmm->ranges_lock, flags);
> /*
> * Since hmm_range_register() holds the mmget() lock hmm_release() is
> * prevented as long as a range exists.
> */
> WARN_ON(!list_empty(&hmm->ranges));
> - mutex_unlock(&hmm->lock);
> + spin_unlock_irqrestore(&hmm->ranges_lock, flags);
>
> down_read(&hmm->mirrors_sem);
> list_for_each_entry(mirror, &hmm->mirrors, list) {
> @@ -151,6 +152,23 @@ static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> hmm_put(hmm);
> }
>
> +static void notifiers_decrement(struct hmm *hmm)
> +{
> + lockdep_assert_held(&hmm->ranges_lock);
> +
> + hmm->notifiers--;
> + if (!hmm->notifiers) {
Nitpick, when doing dec and test or inc and test ops I find it much
easier to read if they are merged into one line, i.e.
if (!--hmm->notifiers) {
Otherwise this looks fine:
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply
* Re: [PATCH 2/2] ipoib: show VF broadcast address
From: Doug Ledford @ 2019-06-15 15:04 UTC (permalink / raw)
To: Denis Kirjanov; +Cc: Denis Kirjanov, davem, netdev, linux-rdma, mkubecek
In-Reply-To: <CAHj3AVny9PijUD7_bWM2-fDNF9n4YZ5xgnG-_O9rZhr1cNVicw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6291 bytes --]
On Sat, 2019-06-15 at 11:49 +0300, Denis Kirjanov wrote:
> On 6/14/19, Doug Ledford <dledford@redhat.com> wrote:
> > On Fri, 2019-06-14 at 15:32 +0200, Denis Kirjanov wrote:
> > > in IPoIB case we can't see a VF broadcast address for but
> > > can see for PF
> > >
> > > Before:
> > > 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc
> > > pfifo_fast
> > > state UP mode DEFAULT group default qlen 256
> > > link/infiniband
> > > 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> > > 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
> > > vf 0 MAC 14:80:00:00:66:fe, spoof checking off, link-state
> > > disable,
> > > trust off, query_rss off
> > > ...
> >
> > The above Before: output should be used as the After: portion of
> > the
> > previous commit message. The previos commit does not fully resolve
> > the
> > problem, but yet the commit message acts as though it does.
> >
> > > After:
> > > 11: ib1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 2044 qdisc
> > > pfifo_fast
> > > state UP mode DEFAULT group default qlen 256
> > > link/infiniband
> > > 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> > > 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff
> > > vf 0 link/infiniband
> > > 80:00:00:66:fe:80:00:00:00:00:00:00:24:8a:07:03:00:a4:3e:7c brd
> > > 00:ff:ff:ff:ff:12:40:1b:ff:ff:00:00:00:00:00:00:ff:ff:ff:ff,
> > > spoof
> > > checking off, link-state disable, trust off, query_rss off
> >
> > Ok, I get why the After: should have a valid broadcast. What I
> > don't
> > get is why the Before: shows a MAC and the After: shows a
> > link/infiniband? What change in this patch is responsible for that
> > difference? I honestly expect, by reading this patch, that you
> > would
> > have a MAC and Broadcast that look like Ethernet, not that the full
> > issue would be resolved.
>
> Hi Doug,
> it's the patch for iproute2 that I'm going to send
Ahh, ok, then please mention that in the commit message or else the
commit message and the patch won't quite make sense together. Maybe do
a before: and after: without the iproute2 patch, then a after-with-
userspace-fix: that is the final result and mention the fix to iproute2
to print out the right link layer and size of address/broadcast.
> > > v1->v2: add the IFLA_VF_BROADCAST constant
> > > v2->v3: put IFLA_VF_BROADCAST at the end
> > > to avoid KABI breakage and set NLA_REJECT
> > > dev_setlink
> > >
> > > Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
> > > ---
> > > include/uapi/linux/if_link.h | 5 +++++
> > > net/core/rtnetlink.c | 5 +++++
> > > 2 files changed, 10 insertions(+)
> > >
> > > diff --git a/include/uapi/linux/if_link.h
> > > b/include/uapi/linux/if_link.h
> > > index 5b225ff63b48..6f75bda2c2d7 100644
> > > --- a/include/uapi/linux/if_link.h
> > > +++ b/include/uapi/linux/if_link.h
> > > @@ -694,6 +694,7 @@ enum {
> > > IFLA_VF_IB_NODE_GUID, /* VF Infiniband node GUID */
> > > IFLA_VF_IB_PORT_GUID, /* VF Infiniband port GUID */
> > > IFLA_VF_VLAN_LIST, /* nested list of vlans, option for
> > > QinQ */
> > > + IFLA_VF_BROADCAST, /* VF broadcast */
> > > __IFLA_VF_MAX,
> > > };
> > >
> > > @@ -704,6 +705,10 @@ struct ifla_vf_mac {
> > > __u8 mac[32]; /* MAX_ADDR_LEN */
> > > };
> > >
> > > +struct ifla_vf_broadcast {
> > > + __u8 broadcast[32];
> > > +};
> > > +
> > > struct ifla_vf_vlan {
> > > __u32 vf;
> > > __u32 vlan; /* 0 - 4095, 0 disables VLAN filter */
> > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > > index cec60583931f..8ac81630ab5c 100644
> > > --- a/net/core/rtnetlink.c
> > > +++ b/net/core/rtnetlink.c
> > > @@ -908,6 +908,7 @@ static inline int rtnl_vfinfo_size(const
> > > struct
> > > net_device *dev,
> > > size += num_vfs *
> > > (nla_total_size(0) +
> > > nla_total_size(sizeof(struct ifla_vf_mac)) +
> > > + nla_total_size(sizeof(struct
> > > ifla_vf_broadcast)) +
> > > nla_total_size(sizeof(struct ifla_vf_vlan)) +
> > > nla_total_size(0) + /* nest IFLA_VF_VLAN_LIST
> > > */
> > > nla_total_size(MAX_VLAN_LIST_LEN *
> > > @@ -1197,6 +1198,7 @@ static noinline_for_stack int
> > > rtnl_fill_vfinfo(struct sk_buff *skb,
> > > struct ifla_vf_vlan vf_vlan;
> > > struct ifla_vf_rate vf_rate;
> > > struct ifla_vf_mac vf_mac;
> > > + struct ifla_vf_broadcast vf_broadcast;
> > > struct ifla_vf_info ivi;
> > >
> > > memset(&ivi, 0, sizeof(ivi));
> > > @@ -1231,6 +1233,7 @@ static noinline_for_stack int
> > > rtnl_fill_vfinfo(struct sk_buff *skb,
> > > vf_trust.vf = ivi.vf;
> > >
> > > memcpy(vf_mac.mac, ivi.mac, sizeof(ivi.mac));
> > > + memcpy(vf_broadcast.broadcast, dev->broadcast, dev->addr_len);
> > > vf_vlan.vlan = ivi.vlan;
> > > vf_vlan.qos = ivi.qos;
> > > vf_vlan_info.vlan = ivi.vlan;
> > > @@ -1247,6 +1250,7 @@ static noinline_for_stack int
> > > rtnl_fill_vfinfo(struct sk_buff *skb,
> > > if (!vf)
> > > goto nla_put_vfinfo_failure;
> > > if (nla_put(skb, IFLA_VF_MAC, sizeof(vf_mac), &vf_mac) ||
> > > + nla_put(skb, IFLA_VF_BROADCAST, sizeof(vf_broadcast),
> > > &vf_broadcast) ||
> > > nla_put(skb, IFLA_VF_VLAN, sizeof(vf_vlan), &vf_vlan) ||
> > > nla_put(skb, IFLA_VF_RATE, sizeof(vf_rate),
> > > &vf_rate) ||
> > > @@ -1753,6 +1757,7 @@ static const struct nla_policy
> > > ifla_info_policy[IFLA_INFO_MAX+1] = {
> > >
> > > static const struct nla_policy ifla_vf_policy[IFLA_VF_MAX+1] = {
> > > [IFLA_VF_MAC] = { .len = sizeof(struct ifla_vf_mac)
> > > },
> > > + [IFLA_VF_BROADCAST] = { .type = NLA_REJECT },
> > > [IFLA_VF_VLAN] = { .len = sizeof(struct
> > > ifla_vf_vlan) },
> > > [IFLA_VF_VLAN_LIST] = { .type = NLA_NESTED },
> > > [IFLA_VF_TX_RATE] = { .len = sizeof(struct ifla_vf_tx_rate)
> > > },
> >
> > --
> > Doug Ledford <dledford@redhat.com>
> > GPG KeyID: B826A3330E572FDD
> > Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
> > 2FDD
> >
>
>
--
Doug Ledford <dledford@redhat.com>
GPG KeyID: B826A3330E572FDD
Key fingerprint = AE6B 1BDA 122B 23B4 265B 1274 B826 A333 0E57
2FDD
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink encap mode
From: Jiri Pirko @ 2019-06-16 10:07 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Parav Pandit, Doug Ledford, Jason Gunthorpe, RDMA mailing list,
Maor Gottlieb, Mark Bloch, Petr Vorel, Saeed Mahameed,
linux-netdev, Jiri Pirko
In-Reply-To: <20190613055954.GV6369@mtr-leonro.mtl.com>
Thu, Jun 13, 2019 at 07:59:54AM CEST, leon@kernel.org wrote:
>On Thu, Jun 13, 2019 at 04:32:25AM +0000, Parav Pandit wrote:
>>
>>
>> > -----Original Message-----
>> > From: Leon Romanovsky <leon@kernel.org>
>> > Sent: Wednesday, June 12, 2019 5:50 PM
>> > To: Doug Ledford <dledford@redhat.com>; Jason Gunthorpe
>> > <jgg@mellanox.com>
>> > Cc: Leon Romanovsky <leonro@mellanox.com>; RDMA mailing list <linux-
>> > rdma@vger.kernel.org>; Maor Gottlieb <maorg@mellanox.com>; Mark Bloch
>> > <markb@mellanox.com>; Parav Pandit <parav@mellanox.com>; Petr Vorel
>> > <pvorel@suse.cz>; Saeed Mahameed <saeedm@mellanox.com>; linux-
>> > netdev <netdev@vger.kernel.org>; Jiri Pirko <jiri@mellanox.com>
>> > Subject: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink
>> > encap mode
>> >
>> > From: Leon Romanovsky <leonro@mellanox.com>
>> >
>> > Devlink has UAPI declaration for encap mode, so there is no need to be
>> > loose on the data get/set by drivers.
>> >
>> > Update call sites to use enum devlink_eswitch_encap_mode instead of plain
>> > u8.
>> >
>> > Suggested-by: Parav Pandit <parav@mellanox.com>
>> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>> > ---
>> > drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 8 +++++---
>> > .../net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 6 ++++--
>> > include/net/devlink.h | 6 ++++--
>> > net/core/devlink.c | 6 ++++--
>> > 4 files changed, 17 insertions(+), 9 deletions(-)
>> >
>> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> > index ed3fad689ec9..e264dfc64a6e 100644
>> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> > @@ -175,7 +175,7 @@ struct mlx5_esw_offload {
>> > DECLARE_HASHTABLE(mod_hdr_tbl, 8);
>> > u8 inline_mode;
>> > u64 num_flows;
>> > - u8 encap;
>> > + enum devlink_eswitch_encap_mode encap;
>> > };
>> >
>> > /* E-Switch MC FDB table hash node */
>> > @@ -356,9 +356,11 @@ int mlx5_devlink_eswitch_inline_mode_set(struct
>> > devlink *devlink, u8 mode,
>> > struct netlink_ext_ack *extack);
>> > int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8
>> > *mode); int mlx5_eswitch_inline_mode_get(struct mlx5_eswitch *esw, int
>> > nvfs, u8 *mode); -int mlx5_devlink_eswitch_encap_mode_set(struct devlink
>> > *devlink, u8 encap,
>> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > encap,
>> > struct netlink_ext_ack *extack);
>> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
>> > *encap);
>> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > *encap);
>> > void *mlx5_eswitch_get_uplink_priv(struct mlx5_eswitch *esw, u8
>> > rep_type);
>> >
>> > int mlx5_eswitch_add_vlan_action(struct mlx5_eswitch *esw, diff --git
>> > a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> > index e09ae27485ee..f1571163143d 100644
>> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> > @@ -2137,7 +2137,8 @@ int mlx5_eswitch_inline_mode_get(struct
>> > mlx5_eswitch *esw, int nvfs, u8 *mode)
>> > return 0;
>> > }
>> >
>> > -int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink, u8
>> > encap,
>> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > encap,
>> > struct netlink_ext_ack *extack)
>> > {
>> > struct mlx5_core_dev *dev = devlink_priv(devlink); @@ -2186,7
>> > +2187,8 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink
>> > *devlink, u8 encap,
>> > return err;
>> > }
>> >
>> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
>> > *encap)
>> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > *encap)
>> > {
>> > struct mlx5_core_dev *dev = devlink_priv(devlink);
>> > struct mlx5_eswitch *esw = dev->priv.eswitch; diff --git
>> > a/include/net/devlink.h b/include/net/devlink.h index
>> > 1c4adfb4195a..7a34fc586def 100644
>> > --- a/include/net/devlink.h
>> > +++ b/include/net/devlink.h
>> > @@ -530,8 +530,10 @@ struct devlink_ops {
>> > int (*eswitch_inline_mode_get)(struct devlink *devlink, u8
>> > *p_inline_mode);
>> > int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> > inline_mode,
>> > struct netlink_ext_ack *extack);
>> > - int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> > *p_encap_mode);
>> > - int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
>> > encap_mode,
>> > + int (*eswitch_encap_mode_get)(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > *p_encap_mode);
>> > + int (*eswitch_encap_mode_set)(struct devlink *devlink,
>> > + enum devlink_eswitch_encap_mode
>> > encap_mode,
>> > struct netlink_ext_ack *extack);
>> > int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
>> > struct netlink_ext_ack *extack);
>> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
>> > d43bc52b8840..47ae69363b07 100644
>> > --- a/net/core/devlink.c
>> > +++ b/net/core/devlink.c
>> > @@ -1552,7 +1552,8 @@ static int devlink_nl_eswitch_fill(struct sk_buff
>> > *msg, struct devlink *devlink,
>> > u32 seq, int flags)
>> > {
>> > const struct devlink_ops *ops = devlink->ops;
>> > - u8 inline_mode, encap_mode;
>> > + enum devlink_eswitch_encap_mode encap_mode;
>> > + u8 inline_mode;
>> > void *hdr;
>> > int err = 0;
>> > u16 mode;
>> > @@ -1628,7 +1629,8 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
>> > sk_buff *skb, {
>> > struct devlink *devlink = info->user_ptr[0];
>> > const struct devlink_ops *ops = devlink->ops;
>> > - u8 inline_mode, encap_mode;
>> > + enum devlink_eswitch_encap_mode encap_mode;
>> > + u8 inline_mode;
>> > int err = 0;
>> > u16 mode;
>> >
>> > --
>> > 2.20.1
>>
>> Netdev follows reverse Christmas tree, but otherwise,
>
>It was before this patch, if Jiri is ok with that, I'll change this
>"const struct devlink_ops *ops = devlink->ops;" line while I'll apply
>this patchset to mlx5-net. If not, I'll leave it as is.
Change to what? I don't follow. The patch looks completely fine to me as
it is.
Acked-by: Jiri Pirko <jiri@mellanox.com>
>
>> Reviewed-by: Parav Pandit <parav@mellanox.com>
>
>Thanks
>
>>
^ permalink raw reply
* Re: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink encap mode
From: Leon Romanovsky @ 2019-06-16 10:15 UTC (permalink / raw)
To: Jiri Pirko
Cc: Parav Pandit, Doug Ledford, Jason Gunthorpe, RDMA mailing list,
Maor Gottlieb, Mark Bloch, Petr Vorel, Saeed Mahameed,
linux-netdev, Jiri Pirko
In-Reply-To: <20190616100707.GB2511@nanopsycho>
On Sun, Jun 16, 2019 at 12:07:07PM +0200, Jiri Pirko wrote:
> Thu, Jun 13, 2019 at 07:59:54AM CEST, leon@kernel.org wrote:
> >On Thu, Jun 13, 2019 at 04:32:25AM +0000, Parav Pandit wrote:
> >>
> >>
> >> > -----Original Message-----
> >> > From: Leon Romanovsky <leon@kernel.org>
> >> > Sent: Wednesday, June 12, 2019 5:50 PM
> >> > To: Doug Ledford <dledford@redhat.com>; Jason Gunthorpe
> >> > <jgg@mellanox.com>
> >> > Cc: Leon Romanovsky <leonro@mellanox.com>; RDMA mailing list <linux-
> >> > rdma@vger.kernel.org>; Maor Gottlieb <maorg@mellanox.com>; Mark Bloch
> >> > <markb@mellanox.com>; Parav Pandit <parav@mellanox.com>; Petr Vorel
> >> > <pvorel@suse.cz>; Saeed Mahameed <saeedm@mellanox.com>; linux-
> >> > netdev <netdev@vger.kernel.org>; Jiri Pirko <jiri@mellanox.com>
> >> > Subject: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink
> >> > encap mode
> >> >
> >> > From: Leon Romanovsky <leonro@mellanox.com>
> >> >
> >> > Devlink has UAPI declaration for encap mode, so there is no need to be
> >> > loose on the data get/set by drivers.
> >> >
> >> > Update call sites to use enum devlink_eswitch_encap_mode instead of plain
> >> > u8.
> >> >
> >> > Suggested-by: Parav Pandit <parav@mellanox.com>
> >> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >> > ---
> >> > drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 8 +++++---
> >> > .../net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 6 ++++--
> >> > include/net/devlink.h | 6 ++++--
> >> > net/core/devlink.c | 6 ++++--
> >> > 4 files changed, 17 insertions(+), 9 deletions(-)
> >> >
> >> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> >> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> >> > index ed3fad689ec9..e264dfc64a6e 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> >> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
> >> > @@ -175,7 +175,7 @@ struct mlx5_esw_offload {
> >> > DECLARE_HASHTABLE(mod_hdr_tbl, 8);
> >> > u8 inline_mode;
> >> > u64 num_flows;
> >> > - u8 encap;
> >> > + enum devlink_eswitch_encap_mode encap;
> >> > };
> >> >
> >> > /* E-Switch MC FDB table hash node */
> >> > @@ -356,9 +356,11 @@ int mlx5_devlink_eswitch_inline_mode_set(struct
> >> > devlink *devlink, u8 mode,
> >> > struct netlink_ext_ack *extack);
> >> > int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8
> >> > *mode); int mlx5_eswitch_inline_mode_get(struct mlx5_eswitch *esw, int
> >> > nvfs, u8 *mode); -int mlx5_devlink_eswitch_encap_mode_set(struct devlink
> >> > *devlink, u8 encap,
> >> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > encap,
> >> > struct netlink_ext_ack *extack);
> >> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
> >> > *encap);
> >> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > *encap);
> >> > void *mlx5_eswitch_get_uplink_priv(struct mlx5_eswitch *esw, u8
> >> > rep_type);
> >> >
> >> > int mlx5_eswitch_add_vlan_action(struct mlx5_eswitch *esw, diff --git
> >> > a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> >> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> >> > index e09ae27485ee..f1571163143d 100644
> >> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> >> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> >> > @@ -2137,7 +2137,8 @@ int mlx5_eswitch_inline_mode_get(struct
> >> > mlx5_eswitch *esw, int nvfs, u8 *mode)
> >> > return 0;
> >> > }
> >> >
> >> > -int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink, u8
> >> > encap,
> >> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > encap,
> >> > struct netlink_ext_ack *extack)
> >> > {
> >> > struct mlx5_core_dev *dev = devlink_priv(devlink); @@ -2186,7
> >> > +2187,8 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink
> >> > *devlink, u8 encap,
> >> > return err;
> >> > }
> >> >
> >> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
> >> > *encap)
> >> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > *encap)
> >> > {
> >> > struct mlx5_core_dev *dev = devlink_priv(devlink);
> >> > struct mlx5_eswitch *esw = dev->priv.eswitch; diff --git
> >> > a/include/net/devlink.h b/include/net/devlink.h index
> >> > 1c4adfb4195a..7a34fc586def 100644
> >> > --- a/include/net/devlink.h
> >> > +++ b/include/net/devlink.h
> >> > @@ -530,8 +530,10 @@ struct devlink_ops {
> >> > int (*eswitch_inline_mode_get)(struct devlink *devlink, u8
> >> > *p_inline_mode);
> >> > int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
> >> > inline_mode,
> >> > struct netlink_ext_ack *extack);
> >> > - int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
> >> > *p_encap_mode);
> >> > - int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
> >> > encap_mode,
> >> > + int (*eswitch_encap_mode_get)(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > *p_encap_mode);
> >> > + int (*eswitch_encap_mode_set)(struct devlink *devlink,
> >> > + enum devlink_eswitch_encap_mode
> >> > encap_mode,
> >> > struct netlink_ext_ack *extack);
> >> > int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
> >> > struct netlink_ext_ack *extack);
> >> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> >> > d43bc52b8840..47ae69363b07 100644
> >> > --- a/net/core/devlink.c
> >> > +++ b/net/core/devlink.c
> >> > @@ -1552,7 +1552,8 @@ static int devlink_nl_eswitch_fill(struct sk_buff
> >> > *msg, struct devlink *devlink,
> >> > u32 seq, int flags)
> >> > {
> >> > const struct devlink_ops *ops = devlink->ops;
> >> > - u8 inline_mode, encap_mode;
> >> > + enum devlink_eswitch_encap_mode encap_mode;
> >> > + u8 inline_mode;
> >> > void *hdr;
> >> > int err = 0;
> >> > u16 mode;
> >> > @@ -1628,7 +1629,8 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
> >> > sk_buff *skb, {
> >> > struct devlink *devlink = info->user_ptr[0];
> >> > const struct devlink_ops *ops = devlink->ops;
> >> > - u8 inline_mode, encap_mode;
> >> > + enum devlink_eswitch_encap_mode encap_mode;
> >> > + u8 inline_mode;
> >> > int err = 0;
> >> > u16 mode;
> >> >
> >> > --
> >> > 2.20.1
> >>
> >> Netdev follows reverse Christmas tree, but otherwise,
> >
> >It was before this patch, if Jiri is ok with that, I'll change this
> >"const struct devlink_ops *ops = devlink->ops;" line while I'll apply
> >this patchset to mlx5-net. If not, I'll leave it as is.
>
> Change to what? I don't follow. The patch looks completely fine to me as
> it is.
Thanks Jiri,
Parav mentioned that two lines above my change were already not in Christmas
tree format.
struct devlink *devlink = info->user_ptr[0];
const struct devlink_ops *ops = devlink->ops;
Thanks
>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
>
>
>
> >
> >> Reviewed-by: Parav Pandit <parav@mellanox.com>
> >
> >Thanks
> >
> >>
^ permalink raw reply
* Re: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink encap mode
From: Jiri Pirko @ 2019-06-16 10:39 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Parav Pandit, Doug Ledford, Jason Gunthorpe, RDMA mailing list,
Maor Gottlieb, Mark Bloch, Petr Vorel, Saeed Mahameed,
linux-netdev, Jiri Pirko
In-Reply-To: <20190616101507.GF4694@mtr-leonro.mtl.com>
Sun, Jun 16, 2019 at 12:15:07PM CEST, leon@kernel.org wrote:
>On Sun, Jun 16, 2019 at 12:07:07PM +0200, Jiri Pirko wrote:
>> Thu, Jun 13, 2019 at 07:59:54AM CEST, leon@kernel.org wrote:
>> >On Thu, Jun 13, 2019 at 04:32:25AM +0000, Parav Pandit wrote:
>> >>
>> >>
>> >> > -----Original Message-----
>> >> > From: Leon Romanovsky <leon@kernel.org>
>> >> > Sent: Wednesday, June 12, 2019 5:50 PM
>> >> > To: Doug Ledford <dledford@redhat.com>; Jason Gunthorpe
>> >> > <jgg@mellanox.com>
>> >> > Cc: Leon Romanovsky <leonro@mellanox.com>; RDMA mailing list <linux-
>> >> > rdma@vger.kernel.org>; Maor Gottlieb <maorg@mellanox.com>; Mark Bloch
>> >> > <markb@mellanox.com>; Parav Pandit <parav@mellanox.com>; Petr Vorel
>> >> > <pvorel@suse.cz>; Saeed Mahameed <saeedm@mellanox.com>; linux-
>> >> > netdev <netdev@vger.kernel.org>; Jiri Pirko <jiri@mellanox.com>
>> >> > Subject: [PATCH mlx5-next v1 1/4] net/mlx5: Declare more strictly devlink
>> >> > encap mode
>> >> >
>> >> > From: Leon Romanovsky <leonro@mellanox.com>
>> >> >
>> >> > Devlink has UAPI declaration for encap mode, so there is no need to be
>> >> > loose on the data get/set by drivers.
>> >> >
>> >> > Update call sites to use enum devlink_eswitch_encap_mode instead of plain
>> >> > u8.
>> >> >
>> >> > Suggested-by: Parav Pandit <parav@mellanox.com>
>> >> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>> >> > ---
>> >> > drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 8 +++++---
>> >> > .../net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 6 ++++--
>> >> > include/net/devlink.h | 6 ++++--
>> >> > net/core/devlink.c | 6 ++++--
>> >> > 4 files changed, 17 insertions(+), 9 deletions(-)
>> >> >
>> >> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> >> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> >> > index ed3fad689ec9..e264dfc64a6e 100644
>> >> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> >> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.h
>> >> > @@ -175,7 +175,7 @@ struct mlx5_esw_offload {
>> >> > DECLARE_HASHTABLE(mod_hdr_tbl, 8);
>> >> > u8 inline_mode;
>> >> > u64 num_flows;
>> >> > - u8 encap;
>> >> > + enum devlink_eswitch_encap_mode encap;
>> >> > };
>> >> >
>> >> > /* E-Switch MC FDB table hash node */
>> >> > @@ -356,9 +356,11 @@ int mlx5_devlink_eswitch_inline_mode_set(struct
>> >> > devlink *devlink, u8 mode,
>> >> > struct netlink_ext_ack *extack);
>> >> > int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8
>> >> > *mode); int mlx5_eswitch_inline_mode_get(struct mlx5_eswitch *esw, int
>> >> > nvfs, u8 *mode); -int mlx5_devlink_eswitch_encap_mode_set(struct devlink
>> >> > *devlink, u8 encap,
>> >> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > encap,
>> >> > struct netlink_ext_ack *extack);
>> >> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
>> >> > *encap);
>> >> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > *encap);
>> >> > void *mlx5_eswitch_get_uplink_priv(struct mlx5_eswitch *esw, u8
>> >> > rep_type);
>> >> >
>> >> > int mlx5_eswitch_add_vlan_action(struct mlx5_eswitch *esw, diff --git
>> >> > a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> >> > b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> >> > index e09ae27485ee..f1571163143d 100644
>> >> > --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> >> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>> >> > @@ -2137,7 +2137,8 @@ int mlx5_eswitch_inline_mode_get(struct
>> >> > mlx5_eswitch *esw, int nvfs, u8 *mode)
>> >> > return 0;
>> >> > }
>> >> >
>> >> > -int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink, u8
>> >> > encap,
>> >> > +int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > encap,
>> >> > struct netlink_ext_ack *extack)
>> >> > {
>> >> > struct mlx5_core_dev *dev = devlink_priv(devlink); @@ -2186,7
>> >> > +2187,8 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink
>> >> > *devlink, u8 encap,
>> >> > return err;
>> >> > }
>> >> >
>> >> > -int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink, u8
>> >> > *encap)
>> >> > +int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > *encap)
>> >> > {
>> >> > struct mlx5_core_dev *dev = devlink_priv(devlink);
>> >> > struct mlx5_eswitch *esw = dev->priv.eswitch; diff --git
>> >> > a/include/net/devlink.h b/include/net/devlink.h index
>> >> > 1c4adfb4195a..7a34fc586def 100644
>> >> > --- a/include/net/devlink.h
>> >> > +++ b/include/net/devlink.h
>> >> > @@ -530,8 +530,10 @@ struct devlink_ops {
>> >> > int (*eswitch_inline_mode_get)(struct devlink *devlink, u8
>> >> > *p_inline_mode);
>> >> > int (*eswitch_inline_mode_set)(struct devlink *devlink, u8
>> >> > inline_mode,
>> >> > struct netlink_ext_ack *extack);
>> >> > - int (*eswitch_encap_mode_get)(struct devlink *devlink, u8
>> >> > *p_encap_mode);
>> >> > - int (*eswitch_encap_mode_set)(struct devlink *devlink, u8
>> >> > encap_mode,
>> >> > + int (*eswitch_encap_mode_get)(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > *p_encap_mode);
>> >> > + int (*eswitch_encap_mode_set)(struct devlink *devlink,
>> >> > + enum devlink_eswitch_encap_mode
>> >> > encap_mode,
>> >> > struct netlink_ext_ack *extack);
>> >> > int (*info_get)(struct devlink *devlink, struct devlink_info_req *req,
>> >> > struct netlink_ext_ack *extack);
>> >> > diff --git a/net/core/devlink.c b/net/core/devlink.c index
>> >> > d43bc52b8840..47ae69363b07 100644
>> >> > --- a/net/core/devlink.c
>> >> > +++ b/net/core/devlink.c
>> >> > @@ -1552,7 +1552,8 @@ static int devlink_nl_eswitch_fill(struct sk_buff
>> >> > *msg, struct devlink *devlink,
>> >> > u32 seq, int flags)
>> >> > {
>> >> > const struct devlink_ops *ops = devlink->ops;
>> >> > - u8 inline_mode, encap_mode;
>> >> > + enum devlink_eswitch_encap_mode encap_mode;
>> >> > + u8 inline_mode;
>> >> > void *hdr;
>> >> > int err = 0;
>> >> > u16 mode;
>> >> > @@ -1628,7 +1629,8 @@ static int devlink_nl_cmd_eswitch_set_doit(struct
>> >> > sk_buff *skb, {
>> >> > struct devlink *devlink = info->user_ptr[0];
>> >> > const struct devlink_ops *ops = devlink->ops;
>> >> > - u8 inline_mode, encap_mode;
>> >> > + enum devlink_eswitch_encap_mode encap_mode;
>> >> > + u8 inline_mode;
>> >> > int err = 0;
>> >> > u16 mode;
>> >> >
>> >> > --
>> >> > 2.20.1
>> >>
>> >> Netdev follows reverse Christmas tree, but otherwise,
>> >
>> >It was before this patch, if Jiri is ok with that, I'll change this
>> >"const struct devlink_ops *ops = devlink->ops;" line while I'll apply
>> >this patchset to mlx5-net. If not, I'll leave it as is.
>>
>> Change to what? I don't follow. The patch looks completely fine to me as
>> it is.
>
>Thanks Jiri,
>
>Parav mentioned that two lines above my change were already not in Christmas
>tree format.
>
> struct devlink *devlink = info->user_ptr[0];
> const struct devlink_ops *ops = devlink->ops;
As there is a dependency between those 2 lines, I don't see how you can
fix this.
>
>Thanks
>
>>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>>
>>
>>
>> >
>> >> Reviewed-by: Parav Pandit <parav@mellanox.com>
>> >
>> >Thanks
>> >
>> >>
^ 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