Re: linux-next: manual merge of the net-next tree with the net tree

[Or Gerlitz]

On Thu, Dec 20, 2018 at 4:47 AM Stephen Rothwell  wrote:
>
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
>   drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
>
> between commit:
>
>   8956f0014ea5 ("net/mlx5e: Fix default amount of channels for VF 
> representors")
>
> from the net tree and commit:
>
>   d9ee0491c2ff ("net/mlx5e: Use dedicated uplink vport netdev representor")
>
> from the net-next tree.
>
> I fixed it up (I just used the net-next tree version) and can carry the
> fix as necessary.  [..]

Yes, this is correct, thanks!

Re: linux-next: manual merge of the net-next tree with the net tree

[Or Gerlitz]

On Mon, Dec 17, 2018 at 11:29 PM Saeed Mahameed
 wrote:
> On Sun, Dec 16, 2018 at 4:25 PM Stephen Rothwell  
> wrote:

> > I fixed it up (see below) and can carry the fix as necessary. This

> Looks good to me.

here too


> > Today's linux-next merge of the net-next tree got a conflict in:
> >   drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > between commit:

> >   154e62abe9cd ("net/mlx5e: Properly initialize flow attributes for slow 
> > path eswitch rule deletion")
> > from the net tree and commit:

> >   e88afe759a49 ("net/mlx5e: Err if asked to mirror a goto chain tc eswitch 
> > rule")
> >   e85e02bad29e ("net/mlx5: E-Switch, Rename esw attr mirror count field")
> > from the net-next tree.

Just a note,

e88afe759a49  ("net/mlx5e: Err if asked to mirror a goto chain tc
eswitch rule")i

s from net  and not net-next

see it here [1] among the top 10 patches

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/log/?qt=grep=mlx5

Re: linux-next: manual merge of the net-next tree with the net tree

[Or Gerlitz]

On Mon, Dec 10, 2018 at 3:38 AM Stephen Rothwell  wrote:
>
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
>   net/sched/cls_flower.c
>
> between commit:
>
>   35cc3cefc4de ("net/sched: cls_flower: Reject duplicated rules also under 
> skip_sw")
>
> from the net tree and commit:
>
>   5c72299fba9d ("net: sched: cls_flower: Classify packets using port ranges")
>
> from the net-next tree.
>
> I fixed it up (see below) and can carry the fix as necessary. This

[..]

The fix LGTM, thanks for addressing this.

Or.

Re: [PATCH] IB/mlx5: avoid binding a new mpi unit to the same devices repeatedly

[Or Gerlitz]

On Sat, Jul 14, 2018 at 2:50 AM, Qing Huang  wrote:
> When a CX5 device is configured in dual-port RoCE mode, after creating
> many VFs against port 1, creating the same number of VFs against port 2
> will flood kernel/syslog with something like
> "mlx5_*:mlx5_ib_bind_slave_port:4266:(pid 5269): port 2 already
> affiliated."
>
> So basically, when traversing mlx5_ib_dev_list, mlx5_ib_add_slave_port()
> shouldn't repeatedly attempt to bind the new mpi data unit to every device
> on the list until it finds an unbound device.

Daniel,

What is mpi data unit?

Or.

>
> Reported-by: Gerald Gibson 
> Signed-off-by: Qing Huang 
> ---
>  drivers/infiniband/hw/mlx5/main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/main.c 
> b/drivers/infiniband/hw/mlx5/main.c
> index b3ba9a2..1ddd1d3 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6068,7 +6068,8 @@ static void *mlx5_ib_add_slave_port(struct 
> mlx5_core_dev *mdev, u8 port_num)
>
> mutex_lock(_ib_multiport_mutex);
> list_for_each_entry(dev, _ib_dev_list, ib_dev_list) {
> -   if (dev->sys_image_guid == mpi->sys_image_guid)
> +   if (dev->sys_image_guid == mpi->sys_image_guid &&
> +   !dev->port[mlx5_core_native_port_num(mdev) - 1].mp.mpi)
> bound = mlx5_ib_bind_slave_port(dev, mpi);
>
> if (bound) {
> --
> 2.9.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

Re: [PATCH] IB/mlx5: avoid binding a new mpi unit to the same devices repeatedly

[Or Gerlitz]

On Sat, Jul 14, 2018 at 2:50 AM, Qing Huang  wrote:
> When a CX5 device is configured in dual-port RoCE mode, after creating
> many VFs against port 1, creating the same number of VFs against port 2
> will flood kernel/syslog with something like
> "mlx5_*:mlx5_ib_bind_slave_port:4266:(pid 5269): port 2 already
> affiliated."
>
> So basically, when traversing mlx5_ib_dev_list, mlx5_ib_add_slave_port()
> shouldn't repeatedly attempt to bind the new mpi data unit to every device
> on the list until it finds an unbound device.

Daniel,

What is mpi data unit?

Or.

>
> Reported-by: Gerald Gibson 
> Signed-off-by: Qing Huang 
> ---
>  drivers/infiniband/hw/mlx5/main.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx5/main.c 
> b/drivers/infiniband/hw/mlx5/main.c
> index b3ba9a2..1ddd1d3 100644
> --- a/drivers/infiniband/hw/mlx5/main.c
> +++ b/drivers/infiniband/hw/mlx5/main.c
> @@ -6068,7 +6068,8 @@ static void *mlx5_ib_add_slave_port(struct 
> mlx5_core_dev *mdev, u8 port_num)
>
> mutex_lock(_ib_multiport_mutex);
> list_for_each_entry(dev, _ib_dev_list, ib_dev_list) {
> -   if (dev->sys_image_guid == mpi->sys_image_guid)
> +   if (dev->sys_image_guid == mpi->sys_image_guid &&
> +   !dev->port[mlx5_core_native_port_num(mdev) - 1].mp.mpi)
> bound = mlx5_ib_bind_slave_port(dev, mpi);
>
> if (bound) {
> --
> 2.9.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

Re: [PATCH 13/14] net: sched: use unique idr insert function in unlocked actions

[Or Gerlitz]

On Mon, May 21, 2018 at 12:33 AM, Marcelo Ricardo Leitner
<marcelo.leit...@gmail.com> wrote:
> On Mon, May 21, 2018 at 12:13:06AM +0300, Or Gerlitz wrote:
>> On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner
>> <marcelo.leit...@gmail.com> wrote:
>> > On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote:
>> >> Substitute calls to action insert function with calls to action insert
>> >> unique function that warns if insertion overwrites index in idr.
>> >
>> > I know this patch may be gone on V2, but a general comment, please use
>> > the function names themselves instead of a textualized version. I.e.,
>> > s/action insert unique/tcf_idr_insert_unique/
>>
>> disagree. While doing reviews I found out that if I ask the developer
>> to use higher
>> level / descriptive language and specifically to avoid putting
>> variable / function names in
>> patch titles and change logs, the quality gets ++ big time, vs if the
>> developer is allowed to say
>>
>> net/mlx5: Changed add_vovo_bobo()
>>
>> Added variable do_it_right to add_vovo_bobo(), now we are terribly good.
>
> In your example I agree that it is not helping and it is even allowing
> such empty changelog, just as in the section I highlighted, the
> descriptive language is also not helping IMHO.
>
> I had to read it 3 times to make sure I wasn't missing a modifier word
> when comparing the two functions and well, it's just saying
> "Substitute calls to foo function to bar function". I don't see how
> the textualized version helps in this case while, at least in this
> one, I would have visually recognized the function names way faster.
>
> Sounds like 2 bad examples for either approach.

Properly written descriptive language is maybe harder to come up with
(specifically for those of us who are not native english speakers, like me)
but is more expressive and helpful for reviews and maintenance. Lets try
to add Vlad to come up with the right higher language and not ask him to
quote function and variable named.

Re: [PATCH 13/14] net: sched: use unique idr insert function in unlocked actions

[Or Gerlitz]

On Mon, May 21, 2018 at 12:33 AM, Marcelo Ricardo Leitner
 wrote:
> On Mon, May 21, 2018 at 12:13:06AM +0300, Or Gerlitz wrote:
>> On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner
>>  wrote:
>> > On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote:
>> >> Substitute calls to action insert function with calls to action insert
>> >> unique function that warns if insertion overwrites index in idr.
>> >
>> > I know this patch may be gone on V2, but a general comment, please use
>> > the function names themselves instead of a textualized version. I.e.,
>> > s/action insert unique/tcf_idr_insert_unique/
>>
>> disagree. While doing reviews I found out that if I ask the developer
>> to use higher
>> level / descriptive language and specifically to avoid putting
>> variable / function names in
>> patch titles and change logs, the quality gets ++ big time, vs if the
>> developer is allowed to say
>>
>> net/mlx5: Changed add_vovo_bobo()
>>
>> Added variable do_it_right to add_vovo_bobo(), now we are terribly good.
>
> In your example I agree that it is not helping and it is even allowing
> such empty changelog, just as in the section I highlighted, the
> descriptive language is also not helping IMHO.
>
> I had to read it 3 times to make sure I wasn't missing a modifier word
> when comparing the two functions and well, it's just saying
> "Substitute calls to foo function to bar function". I don't see how
> the textualized version helps in this case while, at least in this
> one, I would have visually recognized the function names way faster.
>
> Sounds like 2 bad examples for either approach.

Properly written descriptive language is maybe harder to come up with
(specifically for those of us who are not native english speakers, like me)
but is more expressive and helpful for reviews and maintenance. Lets try
to add Vlad to come up with the right higher language and not ask him to
quote function and variable named.

Re: [PATCH 13/14] net: sched: use unique idr insert function in unlocked actions

[Or Gerlitz]

On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner
 wrote:
> On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote:
>> Substitute calls to action insert function with calls to action insert
>> unique function that warns if insertion overwrites index in idr.
>
> I know this patch may be gone on V2, but a general comment, please use
> the function names themselves instead of a textualized version. I.e.,
> s/action insert unique/tcf_idr_insert_unique/

disagree. While doing reviews I found out that if I ask the developer
to use higher
level / descriptive language and specifically to avoid putting
variable / function names in
patch titles and change logs, the quality gets ++ big time, vs if the
developer is allowed to say

net/mlx5: Changed add_vovo_bobo()

Added variable do_it_right to add_vovo_bobo(), now we are terribly good.

Re: [PATCH 13/14] net: sched: use unique idr insert function in unlocked actions

[Or Gerlitz]

On Sun, May 20, 2018 at 1:20 AM, Marcelo Ricardo Leitner
 wrote:
> On Mon, May 14, 2018 at 05:27:14PM +0300, Vlad Buslov wrote:
>> Substitute calls to action insert function with calls to action insert
>> unique function that warns if insertion overwrites index in idr.
>
> I know this patch may be gone on V2, but a general comment, please use
> the function names themselves instead of a textualized version. I.e.,
> s/action insert unique/tcf_idr_insert_unique/

disagree. While doing reviews I found out that if I ask the developer
to use higher
level / descriptive language and specifically to avoid putting
variable / function names in
patch titles and change logs, the quality gets ++ big time, vs if the
developer is allowed to say

net/mlx5: Changed add_vovo_bobo()

Added variable do_it_right to add_vovo_bobo(), now we are terribly good.

Re: Setting large MTU size on slave interfaces may stall the whole system

[Or Gerlitz]

On Tue, Dec 12, 2017 at 5:21 AM, Qing Huang  wrote:
> Hi,
>
> We found an issue with the bonding driver when testing Mellanox devices.
> The following test commands will stall the whole system sometimes, with
> serial console
> flooded with log messages from the bond_miimon_inspect() function. Setting
> mtu size
> to be 1500 seems okay but very rarely it may hit the same problem too.
>
> ip address flush dev ens3f0
> ip link set dev ens3f0 down
> ip address flush dev ens3f1
> ip link set dev ens3f1 down
> [root@ca-hcl629 etc]# modprobe bonding mode=0 miimon=250 use_carrier=1
> updelay=500 downdelay=500
> [root@ca-hcl629 etc]# ifconfig bond0 up
> [root@ca-hcl629 etc]# ifenslave bond0 ens3f0 ens3f1
> [root@ca-hcl629 etc]# ip link set bond0 mtu 4500 up

> Seiral console output:
>
> ** 4 printk messages dropped ** [ 3717.743761] bond0: link status down for
> interface ens3f0, disabling it in 500 ms
[..]


> It seems that when setting a large mtu size on an RoCE interface, the RTNL
> mutex may be held too long by the slave
> interface, causing bond_mii_monitor() to be called repeatedly at an interval
> of 1 tick (1K HZ kernel configuration) and kernel to become unresponsive.

Did you try/managed to reproduce that also with other NIC drivers?

Or.

Re: Setting large MTU size on slave interfaces may stall the whole system

[Or Gerlitz]

On Tue, Dec 12, 2017 at 5:21 AM, Qing Huang  wrote:
> Hi,
>
> We found an issue with the bonding driver when testing Mellanox devices.
> The following test commands will stall the whole system sometimes, with
> serial console
> flooded with log messages from the bond_miimon_inspect() function. Setting
> mtu size
> to be 1500 seems okay but very rarely it may hit the same problem too.
>
> ip address flush dev ens3f0
> ip link set dev ens3f0 down
> ip address flush dev ens3f1
> ip link set dev ens3f1 down
> [root@ca-hcl629 etc]# modprobe bonding mode=0 miimon=250 use_carrier=1
> updelay=500 downdelay=500
> [root@ca-hcl629 etc]# ifconfig bond0 up
> [root@ca-hcl629 etc]# ifenslave bond0 ens3f0 ens3f1
> [root@ca-hcl629 etc]# ip link set bond0 mtu 4500 up

> Seiral console output:
>
> ** 4 printk messages dropped ** [ 3717.743761] bond0: link status down for
> interface ens3f0, disabling it in 500 ms
[..]


> It seems that when setting a large mtu size on an RoCE interface, the RTNL
> mutex may be held too long by the slave
> interface, causing bond_mii_monitor() to be called repeatedly at an interval
> of 1 tick (1K HZ kernel configuration) and kernel to become unresponsive.

Did you try/managed to reproduce that also with other NIC drivers?

Or.

Re: [PATCH] [net-next] net/mlx5e: select CONFIG_MLXFW

[Or Gerlitz]

On Wed, Jun 28, 2017 at 11:10 PM, Arnd Bergmann  wrote:
> With the introduction of mlx5 firmware flash support, we get a link
> error with CONFIG_MLXFW=m and CONFIG_MLX5_CORE=y:
>
> drivers/net/ethernet/mellanox/mlx5/core/fw.o: In function 
> `mlx5_firmware_flash':
> fw.c:(.text+0x9d4): undefined reference to `mlxfw_firmware_flash'

Thanks Arnd, I got a report on that from Jakub but you were before me here..

> We could have a more elaborate method to force MLX5 to be a loadable
> module in this case, but the easiest fix seems to be to always enable
> MLXFW as well, like we do for CONFIG_MLXSW_SPECTRUM, which is the other
> user of mlxfw_firmware_flash.

We would not want to force mlx5 users to build mlxfw.

So lets either use the more elaborate method or maybe instead of using
IS_ENABLED in mlxfw.h use IS_REACHABLE (this was suggested by Jakub)

Or.

Re: [PATCH] [net-next] net/mlx5e: select CONFIG_MLXFW

[Or Gerlitz]

On Wed, Jun 28, 2017 at 11:10 PM, Arnd Bergmann  wrote:
> With the introduction of mlx5 firmware flash support, we get a link
> error with CONFIG_MLXFW=m and CONFIG_MLX5_CORE=y:
>
> drivers/net/ethernet/mellanox/mlx5/core/fw.o: In function 
> `mlx5_firmware_flash':
> fw.c:(.text+0x9d4): undefined reference to `mlxfw_firmware_flash'

Thanks Arnd, I got a report on that from Jakub but you were before me here..

> We could have a more elaborate method to force MLX5 to be a loadable
> module in this case, but the easiest fix seems to be to always enable
> MLXFW as well, like we do for CONFIG_MLXSW_SPECTRUM, which is the other
> user of mlxfw_firmware_flash.

We would not want to force mlx5 users to build mlxfw.

So lets either use the more elaborate method or maybe instead of using
IS_ENABLED in mlxfw.h use IS_REACHABLE (this was suggested by Jakub)

Or.

Re: [PATCH] IB/IPoIB: Check the headroom size

[Or Gerlitz]

On Tue, Apr 25, 2017 at 2:11 PM, Erez Shitrit <ere...@dev.mellanox.co.il> wrote:
> On Tue, Apr 25, 2017 at 1:32 PM, Or Gerlitz <gerlitz...@gmail.com> wrote:
>> On Tue, Apr 25, 2017 at 12:55 PM, Honggang LI <ho...@redhat.com> wrote:
>>> From: Honggang Li <ho...@redhat.com>
>>>
>>> Minimal hard_header_len set by bond_compute_features is ETH_HLEN, which
>>> is smaller than IPOIB_HARD_LEN. ipoib_hard_header should check the
>>> size of headroom to avoid skb_under_panic.
>>
>> sounds terrible, ipoib bonding is supported since ~2007, thanks for
>> reporting on that.
>>
>>> [  122.871493] ipoib_hard_header: skb->head= 8808179d9400, skb->data= 
>>> 8808179d9420, skb_headroom= 0x20
>>> [  123.055400] bond0: Releasing backup interface mthca_ib1
>>> [  123.560529] bond_compute_features:1112 bond0 bond_dev->hard_header_len = 
>>> 14
>>> [  123.568822] CPU: 0 PID: 12336 Comm: ifdown-ib Not tainted 4.9.0-debug #1
>>
>> did you generate this trace by calling dump_stack or this is existing
>> kernel code.
>>
>>> Fixes: fc791b633515 ('IB/ipoib: move back IB LL address into the hard 
>>> header')
>>
>> this is more of WA to avoid some crash or failure but not fixing the
>> actual problem
>>
>> Erez, can you comment?
>
> We saw that after commit fc791b633515, it happened while removing bond
> interface after its slaves (ipoib interface) removed.
> At that point the bond interface sets its dev_harheader_len to be as
> eth interfaces (14 instead of 24), and if a process which doesn't
> aware of the slaves removal or was at the middle of the sending tries
> to send (igmp) packet it goes to ipoib with no space in the skb for
> it, and here comes the panic.

thanks for the info. Is this bug there since ipoib/bonding day one
(and hence my bug...)
or was indeed introduced later? if later, can you explain how
fc791b633515 introduced
that or you only know it by bisection?

> I agree with you that this fix is w/a, and it is a fix in the data
> path for all the packets while the panic is in a control flow. It
> probably should be fixed in the bonding driver.

so what's your suggestion? fc791b633515 is 6m old, and it means the bug
is in stable kernels and probably also in inbox drivers

Or.

Re: [PATCH] IB/IPoIB: Check the headroom size

[Or Gerlitz]

On Tue, Apr 25, 2017 at 2:11 PM, Erez Shitrit  wrote:
> On Tue, Apr 25, 2017 at 1:32 PM, Or Gerlitz  wrote:
>> On Tue, Apr 25, 2017 at 12:55 PM, Honggang LI  wrote:
>>> From: Honggang Li 
>>>
>>> Minimal hard_header_len set by bond_compute_features is ETH_HLEN, which
>>> is smaller than IPOIB_HARD_LEN. ipoib_hard_header should check the
>>> size of headroom to avoid skb_under_panic.
>>
>> sounds terrible, ipoib bonding is supported since ~2007, thanks for
>> reporting on that.
>>
>>> [  122.871493] ipoib_hard_header: skb->head= 8808179d9400, skb->data= 
>>> 8808179d9420, skb_headroom= 0x20
>>> [  123.055400] bond0: Releasing backup interface mthca_ib1
>>> [  123.560529] bond_compute_features:1112 bond0 bond_dev->hard_header_len = 
>>> 14
>>> [  123.568822] CPU: 0 PID: 12336 Comm: ifdown-ib Not tainted 4.9.0-debug #1
>>
>> did you generate this trace by calling dump_stack or this is existing
>> kernel code.
>>
>>> Fixes: fc791b633515 ('IB/ipoib: move back IB LL address into the hard 
>>> header')
>>
>> this is more of WA to avoid some crash or failure but not fixing the
>> actual problem
>>
>> Erez, can you comment?
>
> We saw that after commit fc791b633515, it happened while removing bond
> interface after its slaves (ipoib interface) removed.
> At that point the bond interface sets its dev_harheader_len to be as
> eth interfaces (14 instead of 24), and if a process which doesn't
> aware of the slaves removal or was at the middle of the sending tries
> to send (igmp) packet it goes to ipoib with no space in the skb for
> it, and here comes the panic.

thanks for the info. Is this bug there since ipoib/bonding day one
(and hence my bug...)
or was indeed introduced later? if later, can you explain how
fc791b633515 introduced
that or you only know it by bisection?

> I agree with you that this fix is w/a, and it is a fix in the data
> path for all the packets while the panic is in a control flow. It
> probably should be fixed in the bonding driver.

so what's your suggestion? fc791b633515 is 6m old, and it means the bug
is in stable kernels and probably also in inbox drivers

Or.

Re: [PATCH] IB/IPoIB: Check the headroom size

[Or Gerlitz]

On Tue, Apr 25, 2017 at 12:55 PM, Honggang LI  wrote:
> From: Honggang Li 
>
> Minimal hard_header_len set by bond_compute_features is ETH_HLEN, which
> is smaller than IPOIB_HARD_LEN. ipoib_hard_header should check the
> size of headroom to avoid skb_under_panic.

sounds terrible, ipoib bonding is supported since ~2007, thanks for
reporting on that.

> [  122.871493] ipoib_hard_header: skb->head= 8808179d9400, skb->data= 
> 8808179d9420, skb_headroom= 0x20
> [  123.055400] bond0: Releasing backup interface mthca_ib1
> [  123.560529] bond_compute_features:1112 bond0 bond_dev->hard_header_len = 14
> [  123.568822] CPU: 0 PID: 12336 Comm: ifdown-ib Not tainted 4.9.0-debug #1

did you generate this trace by calling dump_stack or this is existing
kernel code.

> Fixes: fc791b633515 ('IB/ipoib: move back IB LL address into the hard header')

this is more of WA to avoid some crash or failure but not fixing the
actual problem

Erez, can you comment?

Or.

Re: [PATCH] IB/IPoIB: Check the headroom size

[Or Gerlitz]

On Tue, Apr 25, 2017 at 12:55 PM, Honggang LI  wrote:
> From: Honggang Li 
>
> Minimal hard_header_len set by bond_compute_features is ETH_HLEN, which
> is smaller than IPOIB_HARD_LEN. ipoib_hard_header should check the
> size of headroom to avoid skb_under_panic.

sounds terrible, ipoib bonding is supported since ~2007, thanks for
reporting on that.

> [  122.871493] ipoib_hard_header: skb->head= 8808179d9400, skb->data= 
> 8808179d9420, skb_headroom= 0x20
> [  123.055400] bond0: Releasing backup interface mthca_ib1
> [  123.560529] bond_compute_features:1112 bond0 bond_dev->hard_header_len = 14
> [  123.568822] CPU: 0 PID: 12336 Comm: ifdown-ib Not tainted 4.9.0-debug #1

did you generate this trace by calling dump_stack or this is existing
kernel code.

> Fixes: fc791b633515 ('IB/ipoib: move back IB LL address into the hard header')

this is more of WA to avoid some crash or failure but not fixing the
actual problem

Erez, can you comment?

Or.

Re: [PATCH] [net-next] net/mlx5e: fix another maybe-uninitialized false-positive

[Or Gerlitz]

On Fri, Feb 3, 2017 at 6:37 PM, Arnd Bergmann <a...@arndb.de> wrote:
> In commit abeffce ("net/mlx5e: Fix a -Wmaybe-uninitialized warning"), I fixed 
> a
> gcc warning for the ipv4 offload handling. Now we get the same warning for the
> added ipv6 support:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:815:40: warning: 'out_dev' 
> may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> We can apply the same workaround here as well.
>
> Fixes: ce99f6b97fcd ("net/mlx5e: Support SRIOV TC encapsulation offloads for 
> IPv6 tunnels")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

frustrating... I don't see the warning with gcc 5.3.1 [1], but still,
the patch is OKay

Acked-by: Or Gerlitz <ogerl...@mellanox.com>

[1] I used #make KCFLAGS='-Wmaybe-uninitialized'
M=drivers/net/ethernet/mellanox/mlx5/core   -j something

Re: [PATCH] [net-next] net/mlx5e: fix another maybe-uninitialized false-positive

[Or Gerlitz]

On Fri, Feb 3, 2017 at 6:37 PM, Arnd Bergmann  wrote:
> In commit abeffce ("net/mlx5e: Fix a -Wmaybe-uninitialized warning"), I fixed 
> a
> gcc warning for the ipv4 offload handling. Now we get the same warning for the
> added ipv6 support:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:815:40: warning: 'out_dev' 
> may be used uninitialized in this function [-Wmaybe-uninitialized]
>
> We can apply the same workaround here as well.
>
> Fixes: ce99f6b97fcd ("net/mlx5e: Support SRIOV TC encapsulation offloads for 
> IPv6 tunnels")
> Signed-off-by: Arnd Bergmann 

frustrating... I don't see the warning with gcc 5.3.1 [1], but still,
the patch is OKay

Acked-by: Or Gerlitz 

[1] I used #make KCFLAGS='-Wmaybe-uninitialized'
M=drivers/net/ethernet/mellanox/mlx5/core   -j something

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On Thu, Jan 12, 2017 at 6:04 PM, Arnd Bergmann <a...@arndb.de> wrote:
> On Thursday, January 12, 2017 5:21:49 PM CET Or Gerlitz wrote:


>> When I build here without CONFIG_INET in my system, the build goes fine
>> with this approach. However, we're pretty sure that in the past we got
>> 0-day report from the kbuild test robot where he was unhappy that we
>> make the ip_route_output_key call without being wrapped with that #if
>> IS_ENABLED(CONFIG_INET) -- so, we don't want to go there again... thoughts?

> I went back and forth between the two versions, either leaving the #if
> in place, or using the if(IS_ENABLED()) check to be really sure that
> we can't get compile error here.

> I did check that ip_route_output_key() is always declared, but now
> I see that net/route.h might not always be included from en_tc.c
> if CONFIG_INET is disabled (I don't see how it gets included, but
> it obviously is when CONFIG_INET is turned on).

> Adding an explicit include of that file should probably avoid the
> case you ran into earlier, but for I agree it's safer to not rely
> on that here for a bugfix, and just leave the #ifdef. Do you want to
> modify it yourself, or should I spin a new version with that?

I can do that next week, thanks

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On Thu, Jan 12, 2017 at 6:04 PM, Arnd Bergmann  wrote:
> On Thursday, January 12, 2017 5:21:49 PM CET Or Gerlitz wrote:


>> When I build here without CONFIG_INET in my system, the build goes fine
>> with this approach. However, we're pretty sure that in the past we got
>> 0-day report from the kbuild test robot where he was unhappy that we
>> make the ip_route_output_key call without being wrapped with that #if
>> IS_ENABLED(CONFIG_INET) -- so, we don't want to go there again... thoughts?

> I went back and forth between the two versions, either leaving the #if
> in place, or using the if(IS_ENABLED()) check to be really sure that
> we can't get compile error here.

> I did check that ip_route_output_key() is always declared, but now
> I see that net/route.h might not always be included from en_tc.c
> if CONFIG_INET is disabled (I don't see how it gets included, but
> it obviously is when CONFIG_INET is turned on).

> Adding an explicit include of that file should probably avoid the
> case you ran into earlier, but for I agree it's safer to not rely
> on that here for a bugfix, and just leave the #ifdef. Do you want to
> modify it yourself, or should I spin a new version with that?

I can do that next week, thanks

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On 1/11/2017 11:14 PM, Arnd Bergmann wrote:

As found by Olof's build bot, today's mainline kernel gained a harmless
warning about a potential uninitalied variable reference:

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 
'parse_tc_fdb_actions':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:769:13: warning: 'out_dev' may 
be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:811:21: note: 'out_dev' was 
declared here

This was introduced through the addition of an 'IS_ERR/PTR_ERR' pair that
gcc is unfortunately unable to completely figure out. Replacing it with
PTR_ERR_OR_ZERO makes the code more understandable to gcc so it no longer
warns.


can you elaborate on this a little further?


Hadar Hen Zion already attempted to fix the warning earlier by adding
fake initializations, but that ended up just making the code worse without
fully addressing all warnings, so I'm reverting it now that it is no longer 
needed.


ok, so if your approach eliminates the warning on out_dev and also on 
the variables for which Hadar added the faked initializers, I guess we 
should be fine with this change (saw your reply on my other comment), 
just another question:



In order to avoid pulling a variable declaration into the #ifdef, I'm
removing it in favor of a more readable 'if()' statement here that has the same 
effect.


When I build here without CONFIG_INET in my system, the build goes fine 
with this approach. However, we're pretty sure that in the past we got 
0-day report from the kbuild test robot where he was unhappy that we 
make the ip_route_output_key call without being wrapped with that #if 
IS_ENABLED(CONFIG_INET) -- so, we don't want to go there again... thoughts?


Or.

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On 1/11/2017 11:14 PM, Arnd Bergmann wrote:

As found by Olof's build bot, today's mainline kernel gained a harmless
warning about a potential uninitalied variable reference:

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 
'parse_tc_fdb_actions':
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:769:13: warning: 'out_dev' may 
be used uninitialized in this function [-Wmaybe-uninitialized]
drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:811:21: note: 'out_dev' was 
declared here

This was introduced through the addition of an 'IS_ERR/PTR_ERR' pair that
gcc is unfortunately unable to completely figure out. Replacing it with
PTR_ERR_OR_ZERO makes the code more understandable to gcc so it no longer
warns.


can you elaborate on this a little further?


Hadar Hen Zion already attempted to fix the warning earlier by adding
fake initializations, but that ended up just making the code worse without
fully addressing all warnings, so I'm reverting it now that it is no longer 
needed.


ok, so if your approach eliminates the warning on out_dev and also on 
the variables for which Hadar added the faked initializers, I guess we 
should be fine with this change (saw your reply on my other comment), 
just another question:



In order to avoid pulling a variable declaration into the #ifdef, I'm
removing it in favor of a more readable 'if()' statement here that has the same 
effect.


When I build here without CONFIG_INET in my system, the build goes fine 
with this approach. However, we're pretty sure that in the past we got 
0-day report from the kbuild test robot where he was unhappy that we 
make the ip_route_output_key call without being wrapped with that #if 
IS_ENABLED(CONFIG_INET) -- so, we don't want to go there again... thoughts?


Or.

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On 1/11/2017 11:14 PM, Arnd Bergmann wrote:

@@ -666,14 +666,15 @@ static int mlx5e_route_lookup_ipv4(struct mlx5e_priv 
*priv,
struct rtable *rt;
struct neighbour *n = NULL;
int ttl;
+   int ret;
+
+   if (!IS_ENABLED(CONFIG_INET))
+   return -EOPNOTSUPP;
  
-#if IS_ENABLED(CONFIG_INET)

rt = ip_route_output_key(dev_net(mirred_dev), fl4);
-   if (IS_ERR(rt))
-   return PTR_ERR(rt);
-#else
-   return -EOPNOTSUPP;
-#endif
+   ret = PTR_ERR_OR_ZERO(rt);
+   if (ret)
+   return ret;


but this means that if we got NULL from ip_route_output_key, we will 
return success (0) here which is wrong.

Re: [PATCH] [net] net/mlx5e: fix another -Wmaybe-uninitialized warning

[Or Gerlitz]

On 1/11/2017 11:14 PM, Arnd Bergmann wrote:

@@ -666,14 +666,15 @@ static int mlx5e_route_lookup_ipv4(struct mlx5e_priv 
*priv,
struct rtable *rt;
struct neighbour *n = NULL;
int ttl;
+   int ret;
+
+   if (!IS_ENABLED(CONFIG_INET))
+   return -EOPNOTSUPP;
  
-#if IS_ENABLED(CONFIG_INET)

rt = ip_route_output_key(dev_net(mirred_dev), fl4);
-   if (IS_ERR(rt))
-   return PTR_ERR(rt);
-#else
-   return -EOPNOTSUPP;
-#endif
+   ret = PTR_ERR_OR_ZERO(rt);
+   if (ret)
+   return ret;


but this means that if we got NULL from ip_route_output_key, we will 
return success (0) here which is wrong.

Re: [PULL REQUEST] Please pull rdma.git

[Or Gerlitz]

On Fri, Nov 18, 2016 at 4:01 AM, Doug Ledford <dledf...@redhat.com> wrote:
> On 11/17/2016 5:24 PM, Or Gerlitz wrote:

[...]

> I agree with you.  It doesn't fix your patch.  The commit message can
> still be fixed up.

>> Please do not send it to Linus and wait for them to respond. I
>> disagree that it fixes my commit b/c my commit was prior to when
>> route-able RoCE  was introduced and on that time TOS had no relation.

> I agree.  A better fix tag would be the commit that added RoCEv2 support.

But this is the smaller part of the problem. The bigger part is that I
have asked for clarifications on the patch and they didn't provide
anything. So if you are picking patches where a reviewer comments are
ignored, what lesson are you teaching the submitter, that he can just
continue with this practice? why you letting this go that way?

>> does a tiny enhancement for a 10y old commit of Roland, why you think
>> we need it in 4.9-rc6 or 7??

> I don't, it's in the mlx-next branch which means I'll queue it up for
> the 4.10 merge window.  I have no plan on sending that branch for 4.9-rc.

Are you going to comment on that to the submitter? if not, they are
going to continue with this practice.

How are we supposed to realize from patchworks + your github branches
that patches that were submitted for 4.9-rc are picked for 4.10? this
is very confusing and error prone too.

Please comment also on the bunch of patches I pointed you where the
copy you have picked into your tree (pulled it from somewhere?) isn't
what was submitted.

Or.

Re: [PULL REQUEST] Please pull rdma.git

[Or Gerlitz]

On Fri, Nov 18, 2016 at 4:01 AM, Doug Ledford  wrote:
> On 11/17/2016 5:24 PM, Or Gerlitz wrote:

[...]

> I agree with you.  It doesn't fix your patch.  The commit message can
> still be fixed up.

>> Please do not send it to Linus and wait for them to respond. I
>> disagree that it fixes my commit b/c my commit was prior to when
>> route-able RoCE  was introduced and on that time TOS had no relation.

> I agree.  A better fix tag would be the commit that added RoCEv2 support.

But this is the smaller part of the problem. The bigger part is that I
have asked for clarifications on the patch and they didn't provide
anything. So if you are picking patches where a reviewer comments are
ignored, what lesson are you teaching the submitter, that he can just
continue with this practice? why you letting this go that way?

>> does a tiny enhancement for a 10y old commit of Roland, why you think
>> we need it in 4.9-rc6 or 7??

> I don't, it's in the mlx-next branch which means I'll queue it up for
> the 4.10 merge window.  I have no plan on sending that branch for 4.9-rc.

Are you going to comment on that to the submitter? if not, they are
going to continue with this practice.

How are we supposed to realize from patchworks + your github branches
that patches that were submitted for 4.9-rc are picked for 4.10? this
is very confusing and error prone too.

Please comment also on the bunch of patches I pointed you where the
copy you have picked into your tree (pulled it from somewhere?) isn't
what was submitted.

Or.

Re: [PULL REQUEST] Please pull rdma.git

[Or Gerlitz]

On Thu, Nov 17, 2016 at 9:44 PM, Doug Ledford  wrote:
> On 11/17/16 1:49 PM, Leon Romanovsky wrote:
>> On Thu, Nov 17, 2016 at 07:13:54AM -0500, Doug Ledford wrote:
>>> Hi Linus,
>>>
>>> Due to various issues, I've been away and couldn't send a pull request
>>> for about three weeks.  There were a number of -rc patches that built up
>>> in the meantime (some where there already from the early -rc stages).
>>> Obviously, there were way too many to send now, so I tried to pare the
>>> list down to the more important patches for the -rc cycle.

Hi Doug,

Except for the hfi1 patches, all the rest (core, mlx5, mlx4 and rxe)
are marked now as only 21 hours old in your 4.9-rc branch and they
seems be made from you picking partial subsets of multiple series,
with none of them acked by you on the list.

If you agree that I am describing things correctly - how are we
expected to follow on your patch picking? I find it sort of impossible
and error prone.

>> Are you adding the rest to your for-next branch? We would like to have
>> enough time to check that nothing is lost.

> Yes, it's already there in the mlx-next branch on github.

Re the patches there, this one

IB/mlx4: Set traffic class in AH

"Set traffic class within sl_tclass_flowlabel when create iboe AH.
Without this the TOS value will be empty when running VLAN tagged
traffic, because the TOS value is taken from the traffic class in the
address handle attributes.

Fixes: 9106c41 ('IB/mlx4: Fix SL to 802.1Q priority-bits mapping for IBoE')"

claims to fix my commit, I have approached Leon and Co for
clarifications/questions over the list on the patch and nothing was
answered.

Please do not send it to Linus and wait for them to respond. I
disagree that it fixes my commit b/c my commit was prior to when
route-able RoCE  was introduced and on that time TOS had no relation.

and this one

"IB/mlx4: Put non zero value in max_ah device attribute

Use INT_MAX since this is the max value the attribute can hold, though
hardware capability is unlimited.

Fixes: 225c7b1 ('IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters')"

does a tiny enhancement for a 10y old commit of Roland, why you think
we need it in 4.9-rc6 or 7??

Or.

Re: [PULL REQUEST] Please pull rdma.git

[Or Gerlitz]

On Thu, Nov 17, 2016 at 9:44 PM, Doug Ledford  wrote:
> On 11/17/16 1:49 PM, Leon Romanovsky wrote:
>> On Thu, Nov 17, 2016 at 07:13:54AM -0500, Doug Ledford wrote:
>>> Hi Linus,
>>>
>>> Due to various issues, I've been away and couldn't send a pull request
>>> for about three weeks.  There were a number of -rc patches that built up
>>> in the meantime (some where there already from the early -rc stages).
>>> Obviously, there were way too many to send now, so I tried to pare the
>>> list down to the more important patches for the -rc cycle.

Hi Doug,

Except for the hfi1 patches, all the rest (core, mlx5, mlx4 and rxe)
are marked now as only 21 hours old in your 4.9-rc branch and they
seems be made from you picking partial subsets of multiple series,
with none of them acked by you on the list.

If you agree that I am describing things correctly - how are we
expected to follow on your patch picking? I find it sort of impossible
and error prone.

>> Are you adding the rest to your for-next branch? We would like to have
>> enough time to check that nothing is lost.

> Yes, it's already there in the mlx-next branch on github.

Re the patches there, this one

IB/mlx4: Set traffic class in AH

"Set traffic class within sl_tclass_flowlabel when create iboe AH.
Without this the TOS value will be empty when running VLAN tagged
traffic, because the TOS value is taken from the traffic class in the
address handle attributes.

Fixes: 9106c41 ('IB/mlx4: Fix SL to 802.1Q priority-bits mapping for IBoE')"

claims to fix my commit, I have approached Leon and Co for
clarifications/questions over the list on the patch and nothing was
answered.

Please do not send it to Linus and wait for them to respond. I
disagree that it fixes my commit b/c my commit was prior to when
route-able RoCE  was introduced and on that time TOS had no relation.

and this one

"IB/mlx4: Put non zero value in max_ah device attribute

Use INT_MAX since this is the max value the attribute can hold, though
hardware capability is unlimited.

Fixes: 225c7b1 ('IB/mlx4: Add a driver Mellanox ConnectX InfiniBand adapters')"

does a tiny enhancement for a 10y old commit of Roland, why you think
we need it in 4.9-rc6 or 7??

Or.

Re: linux-next: manual merge of the net-next tree with the net tree

[Or Gerlitz]

On Thu, Nov 10, 2016 at 1:50 AM, Stephen Rothwell  wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
>   drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>
> between commit:
>   ee39fbc4447d ("net/mlx5: E-Switch, Set the actions for offloaded rules 
> properly")
> from the net tree and commit:
>   66958ed906b8 ("net/mlx5: Support encap id when setting new steering entry")
> from the net-next tree.

> I fixed it up (see below) and can carry the fix as necessary.

Thanks Stephen, the fix is correct. Dave will hit the conflict the
next time he rebases
net-next on net and will solve it there. Hence the conflict will not
show up in linux-next
once you re-peek net-next.

Or.

> diff --cc drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index d239f5d0ea36,50fe8e8861bb..
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@@ -57,14 -58,14 +58,15 @@@ mlx5_eswitch_add_offloaded_rule(struct
> if (esw->mode != SRIOV_OFFLOADS)
> return ERR_PTR(-EOPNOTSUPP);
>
>  -  flow_act.action = attr->action;
>  +  /* per flow vlan pop/push is emulated, don't set that into the 
> firmware */
> -   action = attr->action & ~(MLX5_FLOW_CONTEXT_ACTION_VLAN_PUSH | 
> MLX5_FLOW_CONTEXT_ACTION_VLAN_POP);
> ++  flow_act.action = attr->action & ~(MLX5_FLOW_CONTEXT_ACTION_VLAN_PUSH 
> | MLX5_FLOW_CONTEXT_ACTION_VLAN_POP);
>
> -   if (action & MLX5_FLOW_CONTEXT_ACTION_FWD_DEST) {
> -   dest.type = MLX5_FLOW_DESTINATION_TYPE_VPORT;
> -   dest.vport_num = attr->out_rep->vport;
> -   action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> -   } else if (action & MLX5_FLOW_CONTEXT_ACTION_COUNT) {
> +   if (flow_act.action & MLX5_FLOW_CONTEXT_ACTION_FWD_DEST) {
> +   dest[i].type = MLX5_FLOW_DESTINATION_TYPE_VPORT;
> +   dest[i].vport_num = attr->out_rep->vport;
> +   i++;
> +   }
> +   if (flow_act.action & MLX5_FLOW_CONTEXT_ACTION_COUNT) {
> counter = mlx5_fc_create(esw->dev, true);
> if (IS_ERR(counter))
> return ERR_CAST(counter);

Re: linux-next: manual merge of the net-next tree with the net tree

[Or Gerlitz]

On Thu, Nov 10, 2016 at 1:50 AM, Stephen Rothwell  wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
>   drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
>
> between commit:
>   ee39fbc4447d ("net/mlx5: E-Switch, Set the actions for offloaded rules 
> properly")
> from the net tree and commit:
>   66958ed906b8 ("net/mlx5: Support encap id when setting new steering entry")
> from the net-next tree.

> I fixed it up (see below) and can carry the fix as necessary.

Thanks Stephen, the fix is correct. Dave will hit the conflict the
next time he rebases
net-next on net and will solve it there. Hence the conflict will not
show up in linux-next
once you re-peek net-next.

Or.

> diff --cc drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> index d239f5d0ea36,50fe8e8861bb..
> --- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
> @@@ -57,14 -58,14 +58,15 @@@ mlx5_eswitch_add_offloaded_rule(struct
> if (esw->mode != SRIOV_OFFLOADS)
> return ERR_PTR(-EOPNOTSUPP);
>
>  -  flow_act.action = attr->action;
>  +  /* per flow vlan pop/push is emulated, don't set that into the 
> firmware */
> -   action = attr->action & ~(MLX5_FLOW_CONTEXT_ACTION_VLAN_PUSH | 
> MLX5_FLOW_CONTEXT_ACTION_VLAN_POP);
> ++  flow_act.action = attr->action & ~(MLX5_FLOW_CONTEXT_ACTION_VLAN_PUSH 
> | MLX5_FLOW_CONTEXT_ACTION_VLAN_POP);
>
> -   if (action & MLX5_FLOW_CONTEXT_ACTION_FWD_DEST) {
> -   dest.type = MLX5_FLOW_DESTINATION_TYPE_VPORT;
> -   dest.vport_num = attr->out_rep->vport;
> -   action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
> -   } else if (action & MLX5_FLOW_CONTEXT_ACTION_COUNT) {
> +   if (flow_act.action & MLX5_FLOW_CONTEXT_ACTION_FWD_DEST) {
> +   dest[i].type = MLX5_FLOW_DESTINATION_TYPE_VPORT;
> +   dest[i].vport_num = attr->out_rep->vport;
> +   i++;
> +   }
> +   if (flow_act.action & MLX5_FLOW_CONTEXT_ACTION_COUNT) {
> counter = mlx5_fc_create(esw->dev, true);
> if (IS_ERR(counter))
> return ERR_CAST(counter);

Re: [PATCH 1/2] net/mlx5e: shut up maybe-uninitialized warning

[Or Gerlitz]

On Fri, Sep 30, 2016 at 7:17 PM, Arnd Bergmann <a...@arndb.de> wrote:
> Build-testing this driver with -Wmaybe-uninitialized gives a new 
> false-positive
> warning that I can't really explain:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 
> 'mlx5e_configure_flower':
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:509:3: error: 'old_attr' may 
> be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> It's obvious from the code that 'old_attr' is initialized whenever 'old'
> is non-NULL here. The warning appears with all versions I tested from gcc-4.7
> through gcc-6.1, and I could not come up with a way to rewrite the function
> in a more readable way that avoids the warning, so I'm adding another
> initialization to shut it up.
>
> Fixes: 8b32580df1cb ("net/mlx5e: Add TC vlan action for SRIOV offloads")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Yeah, this is clearly false positive and I was sure that newish GCCs
don't show that, but you are the master here.. so FWIW

Acked-by: Or Gerlitz <ogerl...@mellanox.com>

Re: [PATCH 1/2] net/mlx5e: shut up maybe-uninitialized warning

[Or Gerlitz]

On Fri, Sep 30, 2016 at 7:17 PM, Arnd Bergmann  wrote:
> Build-testing this driver with -Wmaybe-uninitialized gives a new 
> false-positive
> warning that I can't really explain:
>
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c: In function 
> 'mlx5e_configure_flower':
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:509:3: error: 'old_attr' may 
> be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> It's obvious from the code that 'old_attr' is initialized whenever 'old'
> is non-NULL here. The warning appears with all versions I tested from gcc-4.7
> through gcc-6.1, and I could not come up with a way to rewrite the function
> in a more readable way that avoids the warning, so I'm adding another
> initialization to shut it up.
>
> Fixes: 8b32580df1cb ("net/mlx5e: Add TC vlan action for SRIOV offloads")
> Signed-off-by: Arnd Bergmann 

Yeah, this is clearly false positive and I was sure that newish GCCs
don't show that, but you are the master here.. so FWIW

Acked-by: Or Gerlitz 

Re: [PATCH] net: rtnl: avoid uninitialized data in IFLA_VF_VLAN_LIST handling

[Or Gerlitz]

On Fri, Sep 30, 2016 at 7:13 PM, Arnd Bergmann  wrote:
> With the newly added support for IFLA_VF_VLAN_LIST netlink messages,
> we get a warning about potential uninitialized variable use in
> the parsing of the user input when enabling the -Wmaybe-uninitialized
> warning:
>
> net/core/rtnetlink.c: In function 'do_setvfinfo':
> net/core/rtnetlink.c:1756:9: error: 'ivvl$' may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>
> I have not been able to prove whether it is possible to arrive in
> this code with an empty IFLA_VF_VLAN_LIST block, but if we do,
> then ndo_set_vf_vlan gets called with uninitialized arguments.
>
> This adds an explicit check for an empty list, making it obvious
> to the reader and the compiler that this cannot happen.
>
> Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
Added the authors of the above patch

> Signed-off-by: Arnd Bergmann 

Re: [PATCH] net: rtnl: avoid uninitialized data in IFLA_VF_VLAN_LIST handling

[Or Gerlitz]

On Fri, Sep 30, 2016 at 7:13 PM, Arnd Bergmann  wrote:
> With the newly added support for IFLA_VF_VLAN_LIST netlink messages,
> we get a warning about potential uninitialized variable use in
> the parsing of the user input when enabling the -Wmaybe-uninitialized
> warning:
>
> net/core/rtnetlink.c: In function 'do_setvfinfo':
> net/core/rtnetlink.c:1756:9: error: 'ivvl$' may be used uninitialized in this 
> function [-Werror=maybe-uninitialized]
>
> I have not been able to prove whether it is possible to arrive in
> this code with an empty IFLA_VF_VLAN_LIST block, but if we do,
> then ndo_set_vf_vlan gets called with uninitialized arguments.
>
> This adds an explicit check for an empty list, making it obvious
> to the reader and the compiler that this cannot happen.
>
> Fixes: 79aab093a0b5 ("net: Update API for VF vlan protocol 802.1ad support")
Added the authors of the above patch

> Signed-off-by: Arnd Bergmann 

Re: [patch] devlink: clean up a condition

[Or Gerlitz]

On 6/16/2016 9:50 AM, Dan Carpenter wrote:

Presumably having a _get() function implies that we also have a _set()
function but lets make it match when we're calling.

Signed-off-by: Dan Carpenter 

diff --git a/net/core/devlink.c b/net/core/devlink.c
index a4f88cb..b2e592a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1461,7 +1461,7 @@ static int devlink_nl_cmd_eswitch_mode_set_doit(struct 
sk_buff *skb,
  
  	mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
  
-	if (ops && ops->eswitch_mode_get)

+   if (ops && ops->eswitch_mode_set)
return ops->eswitch_mode_set(devlink, mode);
return -EOPNOTSUPP;
  }



good catch Dan, we will incorporate that into the patch set

Re: [patch] devlink: clean up a condition

[Or Gerlitz]

On 6/16/2016 9:50 AM, Dan Carpenter wrote:

Presumably having a _get() function implies that we also have a _set()
function but lets make it match when we're calling.

Signed-off-by: Dan Carpenter 

diff --git a/net/core/devlink.c b/net/core/devlink.c
index a4f88cb..b2e592a 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1461,7 +1461,7 @@ static int devlink_nl_cmd_eswitch_mode_set_doit(struct 
sk_buff *skb,
  
  	mode = nla_get_u16(info->attrs[DEVLINK_ATTR_ESWITCH_MODE]);
  
-	if (ops && ops->eswitch_mode_get)

+   if (ops && ops->eswitch_mode_set)
return ops->eswitch_mode_set(devlink, mode);
return -EOPNOTSUPP;
  }



good catch Dan, we will incorporate that into the patch set

Re: [PATCH net-next v3 6/7] vmxnet3: introduce command to register memory region

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:

> +++ b/drivers/net/vmxnet3/vmxnet3_defs.h
> @@ -81,6 +81,7 @@ enum {
> VMXNET3_CMD_RESERVED2,
> VMXNET3_CMD_RESERVED3,
> VMXNET3_CMD_SET_COALESCE,
> +   VMXNET3_CMD_REGISTER_MEMREGS,
>
> VMXNET3_CMD_FIRST_GET = 0xF00D,
> VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> @@ -668,6 +669,22 @@ struct Vmxnet3_CoalesceScheme {
> } coalPara;
>  };
>
> +struct Vmxnet3_MemoryRegion {
> +   __le64  startPA;
> +   __le32  length;
> +   __le16  txQueueBits;
> +   __le16  rxQueueBits;
> +};


What is the use case for this command?

What's the role of the tx/rx queue bits fields?

Re: [PATCH net-next v3 6/7] vmxnet3: introduce command to register memory region

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:

> +++ b/drivers/net/vmxnet3/vmxnet3_defs.h
> @@ -81,6 +81,7 @@ enum {
> VMXNET3_CMD_RESERVED2,
> VMXNET3_CMD_RESERVED3,
> VMXNET3_CMD_SET_COALESCE,
> +   VMXNET3_CMD_REGISTER_MEMREGS,
>
> VMXNET3_CMD_FIRST_GET = 0xF00D,
> VMXNET3_CMD_GET_QUEUE_STATUS = VMXNET3_CMD_FIRST_GET,
> @@ -668,6 +669,22 @@ struct Vmxnet3_CoalesceScheme {
> } coalPara;
>  };
>
> +struct Vmxnet3_MemoryRegion {
> +   __le64  startPA;
> +   __le32  length;
> +   __le16  txQueueBits;
> +   __le16  rxQueueBits;
> +};


What is the use case for this command?

What's the role of the tx/rx queue bits fields?

Re: [PATCH net-next v3 0/7] vmxnet3: upgrade to version 3

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:
> This patchset upgrades vmxnet3 to version 3.

Except for one patch, all the rest lack change-log, which makes it
somehow needlessly harder to review and maintain

> Shrikrishna Khare (7):
>   vmxnet3: prepare for version 3 changes
>   vmxnet3: introduce generic command interface to configure the device
>   vmxnet3: allow variable length transmit data ring buffer
>   vmxnet3: add receive data ring support
>   vmxnet3: add support for get_coalesce, set_coalesce ethtool operations
>   vmxnet3: introduce command to register memory region
>   vmxnet3: update to version 3

Re: [PATCH net-next v3 0/7] vmxnet3: upgrade to version 3

[Or Gerlitz]

On Tue, Jun 14, 2016 at 4:50 AM, Shrikrishna Khare  wrote:
> This patchset upgrades vmxnet3 to version 3.

Except for one patch, all the rest lack change-log, which makes it
somehow needlessly harder to review and maintain

> Shrikrishna Khare (7):
>   vmxnet3: prepare for version 3 changes
>   vmxnet3: introduce generic command interface to configure the device
>   vmxnet3: allow variable length transmit data ring buffer
>   vmxnet3: add receive data ring support
>   vmxnet3: add support for get_coalesce, set_coalesce ethtool operations
>   vmxnet3: introduce command to register memory region
>   vmxnet3: update to version 3

Re: [PATCH v5 09/21] IB/hns: Add hca support

[Or Gerlitz]

On Tue, May 3, 2016 at 10:57 AM, Wei Hu (Xavier)
<xavier.hu...@huawei.com> wrote:
> On 2016/4/30 12:33, Or Gerlitz wrote:


>> Can you elaborate what design aspects in the driver or anywhere else
>> should impose that limitation?

> 1.  Oulijun resolved the problem, and sent PATCH V6 on 2016-4-28. Thanks for
> more comments.

V6 still conditions things on ARM


> 2.  This driver for Hisilicon RoCE Engine run in ARM SoCs.
> The Hisilicon Network Subsystem is a long term evolution IP which is
> supposed to be used in Hisilicon ICT SoCs. HNS(Hisilicon Network Subsystem)
> has a hardware support of performing RDMA with RoCE engine.

I understand that the HW is targeted just for ARM environments. Is
this the reason
why you want to impose this build limitation or there's something in
the driver design
or code that is not going to work in other environments?

> This Kconfig related with this driver as below:
>
> config INFINIBAND_HISILICON_HNS
> tristate "Hisilicon Hns ROCE Driver"
> depends on NET_VENDOR_HISILICON
> depends on ARM64 && HNS && HNS_DSAF && HNS_ENET

this is understood, my question came to better understand the limitations

Re: [PATCH v5 09/21] IB/hns: Add hca support

[Or Gerlitz]

On Tue, May 3, 2016 at 10:57 AM, Wei Hu (Xavier)
 wrote:
> On 2016/4/30 12:33, Or Gerlitz wrote:


>> Can you elaborate what design aspects in the driver or anywhere else
>> should impose that limitation?

> 1.  Oulijun resolved the problem, and sent PATCH V6 on 2016-4-28. Thanks for
> more comments.

V6 still conditions things on ARM


> 2.  This driver for Hisilicon RoCE Engine run in ARM SoCs.
> The Hisilicon Network Subsystem is a long term evolution IP which is
> supposed to be used in Hisilicon ICT SoCs. HNS(Hisilicon Network Subsystem)
> has a hardware support of performing RDMA with RoCE engine.

I understand that the HW is targeted just for ARM environments. Is
this the reason
why you want to impose this build limitation or there's something in
the driver design
or code that is not going to work in other environments?

> This Kconfig related with this driver as below:
>
> config INFINIBAND_HISILICON_HNS
> tristate "Hisilicon Hns ROCE Driver"
> depends on NET_VENDOR_HISILICON
> depends on ARM64 && HNS && HNS_DSAF && HNS_ENET

this is understood, my question came to better understand the limitations

Re: [PATCH v5 09/21] IB/hns: Add hca support

[Or Gerlitz]

On Wed, Apr 27, 2016 at 6:34 AM, oulijun  wrote:
> On 2016/4/26 22:25, Jiri Pirko wrote:
>> Tue, Apr 26, 2016 at 04:18:21PM CEST, l...@kernel.org wrote:

 I appreciate your keen eye. this code is meant for ARM64bit therefore 
 should run corretly for 64-bit AARCH64.

>> The driver should run correctly on any arch.

>  Hi Jiri Pirko,
> Our driver run in ARM64 platform by depending on Kconfig. It will be 
> configure in Kconfig file.

Can you elaborate what design aspects in the driver or anywhere else
should impose that limitation?

Or.

Re: [PATCH v5 09/21] IB/hns: Add hca support

[Or Gerlitz]

On Wed, Apr 27, 2016 at 6:34 AM, oulijun  wrote:
> On 2016/4/26 22:25, Jiri Pirko wrote:
>> Tue, Apr 26, 2016 at 04:18:21PM CEST, l...@kernel.org wrote:

 I appreciate your keen eye. this code is meant for ARM64bit therefore 
 should run corretly for 64-bit AARCH64.

>> The driver should run correctly on any arch.

>  Hi Jiri Pirko,
> Our driver run in ARM64 platform by depending on Kconfig. It will be 
> configure in Kconfig file.

Can you elaborate what design aspects in the driver or anywhere else
should impose that limitation?

Or.

Re: linux-next: manual merge of the rdma tree with the net-next tree

[Or Gerlitz]

On Wed, Mar 16, 2016 at 7:44 PM, Linus Torvalds
<torva...@linux-foundation.org> wrote:
> On Wed, Mar 16, 2016 at 10:35 AM, Doug Ledford <dledf...@redhat.com> wrote:
>> On 3/16/2016 1:18 PM, Linus Torvalds wrote:
>>> On Tue, Mar 15, 2016 at 5:58 PM, Stephen Rothwell <s...@canb.auug.org.au> 
>>> wrote:

>>>> I fixed it up (see below) and can carry the fix as necessary (no action
>>>> is required).

>>> Side note: can you change this wording for your manual merge script?
>>> Last merge window (or was it the one before it?) we had confusion with
>>> people who thought that "no action is required" means "you can just
>>> ignore this entirely".

>> I certainly didn't take it that way regardless of the wording.

> It was Or Gerlitz. You were cc'd, since it was the whole rdma Mellanox
> mess. I quote from that thread:
>
>  "> However, the fact that it got resolved in linux-next is purely
>   > informational. It doesn't "fix" the conflict - it just means that both
>   > sides should have gotten informed about it. That doesn't mean that the
>   > conflict goes away or becomes better.
>
>   That's news to me. When such things happen and caught by Stephen, we
>   are getting an email saying something like
>
>   "Today's linux-next merge of the infiniband tree got a conflict
>   between commit X from net-next tree and commit Y from the infiniband
>   tree. I fixed it up (see below) and can carry the fix as necessary (no
>   action is required)."
>
>   Also asked around a bit and got to learn on Stephen using git rerere,
>   so all (no action needed note + seeing git rerere in action...) that
>   leaded me to think that indeed no action is required from our side,
>   but after reading your email (twice, so far), I realized that this was
>   wrong conclusion."

> So that whole "no action is required" wording very much has caused
> confusion before in the rdma camp.

> Let's fix the wording. I'm indeed hopeful that the rdma camp is now
> keenly aware of the issues, but that doesn't change the fact that the
> wording has been problematic.

>> "[...] The Mellanox people are on my xxit-list until they show that they can
>> actually act like responsible people [...]"

Linus,

As I wrote you in that other thread you were quoting from there,
following to the happenings mentioned there, we took responsibility
and made bunch of corrective actions within Mellanox which got us to a
point where there was only one rdma/netdev-next conflict for the 4.6
merge window.

I know there's history here, and in the 4.5 cycle things were much
worse, but I still wanted to put things in their more precise place,
if you don't mind.

Or.

Re: linux-next: manual merge of the rdma tree with the net-next tree

[Or Gerlitz]

On Wed, Mar 16, 2016 at 7:44 PM, Linus Torvalds
 wrote:
> On Wed, Mar 16, 2016 at 10:35 AM, Doug Ledford  wrote:
>> On 3/16/2016 1:18 PM, Linus Torvalds wrote:
>>> On Tue, Mar 15, 2016 at 5:58 PM, Stephen Rothwell  
>>> wrote:

>>>> I fixed it up (see below) and can carry the fix as necessary (no action
>>>> is required).

>>> Side note: can you change this wording for your manual merge script?
>>> Last merge window (or was it the one before it?) we had confusion with
>>> people who thought that "no action is required" means "you can just
>>> ignore this entirely".

>> I certainly didn't take it that way regardless of the wording.

> It was Or Gerlitz. You were cc'd, since it was the whole rdma Mellanox
> mess. I quote from that thread:
>
>  "> However, the fact that it got resolved in linux-next is purely
>   > informational. It doesn't "fix" the conflict - it just means that both
>   > sides should have gotten informed about it. That doesn't mean that the
>   > conflict goes away or becomes better.
>
>   That's news to me. When such things happen and caught by Stephen, we
>   are getting an email saying something like
>
>   "Today's linux-next merge of the infiniband tree got a conflict
>   between commit X from net-next tree and commit Y from the infiniband
>   tree. I fixed it up (see below) and can carry the fix as necessary (no
>   action is required)."
>
>   Also asked around a bit and got to learn on Stephen using git rerere,
>   so all (no action needed note + seeing git rerere in action...) that
>   leaded me to think that indeed no action is required from our side,
>   but after reading your email (twice, so far), I realized that this was
>   wrong conclusion."

> So that whole "no action is required" wording very much has caused
> confusion before in the rdma camp.

> Let's fix the wording. I'm indeed hopeful that the rdma camp is now
> keenly aware of the issues, but that doesn't change the fact that the
> wording has been problematic.

>> "[...] The Mellanox people are on my xxit-list until they show that they can
>> actually act like responsible people [...]"

Linus,

As I wrote you in that other thread you were quoting from there,
following to the happenings mentioned there, we took responsibility
and made bunch of corrective actions within Mellanox which got us to a
point where there was only one rdma/netdev-next conflict for the 4.6
merge window.

I know there's history here, and in the 4.5 cycle things were much
worse, but I still wanted to put things in their more precise place,
if you don't mind.

Or.

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Thu, Mar 17, 2016 at 3:40 AM, Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> On 03/16/2016 08:45 PM, Or Gerlitz wrote:
>> On Wed, Mar 16, 2016 at 10:34 AM, Alexey Kardashevskiy <a...@ozlabs.ru>
>> wrote:
>>
>>> Oh. ok. It also looks like even with the reverted patch, mlx4 VF does not
>>> work in a guest:
>>
>>
>> So where is the breakage point for you? does 4.4 works? if not, what?

> Ah, my bad. It is unrelated to the kernel version.
> I tried passing a PF to a guest while its VFs are already passed to another
> guest and see how exactly it blows up (AER/EEH were thrown but the host
> recovered => good) but this left the device in a weird state when I could
> not use VF in a guest anymore but it seemed to keep working on the host.

> It seems like the actual adapter does not reset completely when the machine
> is rebooted, I had unplug/replug power cables to fix this.

So to make sure, now things works fine with the patch reverted?

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Thu, Mar 17, 2016 at 3:40 AM, Alexey Kardashevskiy  wrote:
> On 03/16/2016 08:45 PM, Or Gerlitz wrote:
>> On Wed, Mar 16, 2016 at 10:34 AM, Alexey Kardashevskiy 
>> wrote:
>>
>>> Oh. ok. It also looks like even with the reverted patch, mlx4 VF does not
>>> work in a guest:
>>
>>
>> So where is the breakage point for you? does 4.4 works? if not, what?

> Ah, my bad. It is unrelated to the kernel version.
> I tried passing a PF to a guest while its VFs are already passed to another
> guest and see how exactly it blows up (AER/EEH were thrown but the host
> recovered => good) but this left the device in a weird state when I could
> not use VF in a guest anymore but it seemed to keep working on the host.

> It seems like the actual adapter does not reset completely when the machine
> is rebooted, I had unplug/replug power cables to fix this.

So to make sure, now things works fine with the patch reverted?

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Wed, Mar 16, 2016 at 10:34 AM, Alexey Kardashevskiy  wrote:

> Oh. ok. It also looks like even with the reverted patch, mlx4 VF does not
> work in a guest:

So where is the breakage point for you? does 4.4 works? if not, what?

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Wed, Mar 16, 2016 at 10:34 AM, Alexey Kardashevskiy  wrote:

> Oh. ok. It also looks like even with the reverted patch, mlx4 VF does not
> work in a guest:

So where is the breakage point for you? does 4.4 works? if not, what?

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Tue, Mar 15, 2016 at 2:18 PM, Christoph Hellwig <h...@infradead.org> wrote:
> On Tue, Mar 15, 2016 at 12:40:06PM +0200, Or Gerlitz wrote:
>> "[..] Regarding backward compatibility in SR-IOV, if hypervisor has
>> this new code, the virtual OS must be updated. [...]"

> Which is broken, we can't break user or guest VM ABIs ever.

Let us check. I was under (the maybe wrong) impression, that before this
patch both PF/VF drivers were not operative on some systems, so on those
systems it's fair to require the VF driver to be patched too.

Or.

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Tue, Mar 15, 2016 at 2:18 PM, Christoph Hellwig  wrote:
> On Tue, Mar 15, 2016 at 12:40:06PM +0200, Or Gerlitz wrote:
>> "[..] Regarding backward compatibility in SR-IOV, if hypervisor has
>> this new code, the virtual OS must be updated. [...]"

> Which is broken, we can't break user or guest VM ABIs ever.

Let us check. I was under (the maybe wrong) impression, that before this
patch both PF/VF drivers were not operative on some systems, so on those
systems it's fair to require the VF driver to be patched too.

Or.

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Tue, Mar 15, 2016 at 12:19 PM, Alexey Kardashevskiy  wrote:
> This reverts commit 85743f1eb34548ba4b056d2f184a3d107a3b8917.
>
> Without this revert, POWER "pseries" KVM guests with a VF passed to a guest
> using VFIO fail to bring the driver up:
>
> mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014)
> mlx4_core: Initializing :00:00.0
> mlx4_core :00:00.0: enabling device ( -> 0002)
> mlx4_core :00:00.0: Detected virtual function - running in slave mode
> mlx4_core :00:00.0: Sending reset
> mlx4_core :00:00.0: Sending vhcr0
> mlx4_core :00:00.0: HCA minimum page size:512
> mlx4_core :00:00.0: UAR size:4096 != kernel PAGE_SIZE of 65536
> mlx4_core :00:00.0: Failed to obtain slave caps

> Both host and guest use 64K system pages.
>
> How to fix this properly? Thanks.

The commit message says:

"[..] Regarding backward compatibility in SR-IOV, if hypervisor has
this new code, the virtual OS must be updated. [...]"


Or.

Re: [RFC PATCH kernel] Revert "net/mlx4_core: Set UAR page size to 4KB regardless of system page size"

[Or Gerlitz]

On Tue, Mar 15, 2016 at 12:19 PM, Alexey Kardashevskiy  wrote:
> This reverts commit 85743f1eb34548ba4b056d2f184a3d107a3b8917.
>
> Without this revert, POWER "pseries" KVM guests with a VF passed to a guest
> using VFIO fail to bring the driver up:
>
> mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014)
> mlx4_core: Initializing :00:00.0
> mlx4_core :00:00.0: enabling device ( -> 0002)
> mlx4_core :00:00.0: Detected virtual function - running in slave mode
> mlx4_core :00:00.0: Sending reset
> mlx4_core :00:00.0: Sending vhcr0
> mlx4_core :00:00.0: HCA minimum page size:512
> mlx4_core :00:00.0: UAR size:4096 != kernel PAGE_SIZE of 65536
> mlx4_core :00:00.0: Failed to obtain slave caps

> Both host and guest use 64K system pages.
>
> How to fix this properly? Thanks.

The commit message says:

"[..] Regarding backward compatibility in SR-IOV, if hypervisor has
this new code, the virtual OS must be updated. [...]"


Or.

Re: [PATCH RESEND] infiniband:core:Fix error handling in the function cm_lap_handler

[Or Gerlitz]

On 2/22/2016 8:59 PM, Nicholas Krause wrote:

This fixes error handling in the function cm_lap_handler to properly
check if the internal call to the function cm_init_av_for_response
has failed by returning a error code and if so exit immediately from
this particular function by freeing all previously allocated
resources before returning the error code to this function's caller
in order to allow the caller to handle the error properly in it's

Signed-off-by: Nicholas Krause


I think this is the longest sentence I have ever read in English, and it 
even doesn't have a end... please break it and tell us the (hopefully) 
happy end.


Use IB/core: prefix and point to the commit you are fixing

Fixes: xxx ('yyy')

where xxx is the 12 digit abbrev-iated git short log  and yyy the commit 
title

Re: [PATCH RESEND] infiniband:core:Fix error handling in the function cm_lap_handler

[Or Gerlitz]

On 2/22/2016 8:59 PM, Nicholas Krause wrote:

This fixes error handling in the function cm_lap_handler to properly
check if the internal call to the function cm_init_av_for_response
has failed by returning a error code and if so exit immediately from
this particular function by freeing all previously allocated
resources before returning the error code to this function's caller
in order to allow the caller to handle the error properly in it's

Signed-off-by: Nicholas Krause


I think this is the longest sentence I have ever read in English, and it 
even doesn't have a end... please break it and tell us the (hopefully) 
happy end.


Use IB/core: prefix and point to the commit you are fixing

Fixes: xxx ('yyy')

where xxx is the 12 digit abbrev-iated git short log  and yyy the commit 
title

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 5:20 PM, Chuck Lever wrote:

Chuck,
>
>Lets be concrete... anything wrong with patch [1]?

Yes. It is missing Acked-by: lines from the maintainers of
those files.

All changes to files under net/sunrpc need an Ack from one
of the maintainers listed in MAINTAINERS for that directory,
if the changes are going through another maintainer's tree.

I have been personally asked to remind folks that the
nfs-sunrpc maintainers do not read linux-rdma, so they
must be contacted directly (and cc: linux-nfs) as part of
proposing finished patches in that area.


I did that!!

I copied you and Anna on the patch [1].

Again, lets be concrete, this very small cleanup was picked and merged, 
anything there

need to be fixed?

Or.

[1] http://marc.info/?l=linux-rdma=145042924110411=2



Unfortunately I have not been able to review every patch
that has come by on linux-rdma in the past 9 months to
ensure the eyes are dotted and tees crossed. More than
a few commits in the tree are missing the proper tags.


>[1]  commit e3e45b1 "xprtrdma: Avoid calling ib_query_device"
>
>in git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git k.o/for-4.5
>
>http://git.kernel.org/cgit/linux/kernel/git/dledford/rdma.git/commit/?h=k.o/for-4.5=e3e45b1b43988b99007a9908ca0ba738b3fbd0ff
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 4:24 PM, Chuck Lever wrote:

Actually, one of Or's for-4.5 devattr patches doesn't appear to have the proper 
Ack's for the changes under net/sunrpc/xprtrdma either.


Chuck,

Lets be concrete... anything wrong with patch [1]?

Or.

[1]  commit e3e45b1 "xprtrdma: Avoid calling ib_query_device"

in git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git 
k.o/for-4.5


http://git.kernel.org/cgit/linux/kernel/git/dledford/rdma.git/commit/?h=k.o/for-4.5=e3e45b1b43988b99007a9908ca0ba738b3fbd0ff

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 2:15 PM, Christoph Hellwig wrote:

I'm not Doug, but all the recent for 4.5 work is in Dougs tree
at

https://github.com/dledford/linux  rdma/k.o/for-4.5


Christoph,

As I wrote here, the bits are already @ kernel.org

git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git k.o/for-4.5

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 2:01 PM, Chuck Lever wrote:

what is the URL for the branch I should rebase on?


k.o/for-4.5 on Doug's kernel.org tree
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 2:15 PM, Christoph Hellwig wrote:

I'm not Doug, but all the recent for 4.5 work is in Dougs tree
at

https://github.com/dledford/linux  rdma/k.o/for-4.5


Christoph,

As I wrote here, the bits are already @ kernel.org

git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git k.o/for-4.5

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 2:01 PM, Chuck Lever wrote:

what is the URL for the branch I should rebase on?


k.o/for-4.5 on Doug's kernel.org tree
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 4:24 PM, Chuck Lever wrote:

Actually, one of Or's for-4.5 devattr patches doesn't appear to have the proper 
Ack's for the changes under net/sunrpc/xprtrdma either.


Chuck,

Lets be concrete... anything wrong with patch [1]?

Or.

[1]  commit e3e45b1 "xprtrdma: Avoid calling ib_query_device"

in git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git 
k.o/for-4.5


http://git.kernel.org/cgit/linux/kernel/git/dledford/rdma.git/commit/?h=k.o/for-4.5=e3e45b1b43988b99007a9908ca0ba738b3fbd0ff

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the nfsd tree

[Or Gerlitz]

On 1/6/2016 5:20 PM, Chuck Lever wrote:

Chuck,
>
>Lets be concrete... anything wrong with patch [1]?

Yes. It is missing Acked-by: lines from the maintainers of
those files.

All changes to files under net/sunrpc need an Ack from one
of the maintainers listed in MAINTAINERS for that directory,
if the changes are going through another maintainer's tree.

I have been personally asked to remind folks that the
nfs-sunrpc maintainers do not read linux-rdma, so they
must be contacted directly (and cc: linux-nfs) as part of
proposing finished patches in that area.


I did that!!

I copied you and Anna on the patch [1].

Again, lets be concrete, this very small cleanup was picked and merged, 
anything there

need to be fixed?

Or.

[1] http://marc.info/?l=linux-rdma=145042924110411=2



Unfortunately I have not been able to review every patch
that has come by on linux-rdma in the past 9 months to
ensure the eyes are dotted and tees crossed. More than
a few commits in the tree are missing the proper tags.


>[1]  commit e3e45b1 "xprtrdma: Avoid calling ib_query_device"
>
>in git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git k.o/for-4.5
>
>http://git.kernel.org/cgit/linux/kernel/git/dledford/rdma.git/commit/?h=k.o/for-4.5=e3e45b1b43988b99007a9908ca0ba738b3fbd0ff
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the net-next tree

[Or Gerlitz]

On 1/5/2016 3:51 AM, Stephen Rothwell wrote:

Hi Doug,

Today's linux-next merge of the rdma tree got conflicts in:

   drivers/net/ethernet/mellanox/mlx5/core/vport.c
   include/linux/mlx5/mlx5_ifc.h
   include/linux/mlx5/vport.h

between commits:

   e1d7d349c69d ("net/mlx5: Update access functions to Query/Modify vport MAC 
address")
   e75465148b7d ("net/mlx5: Introduce access functions to modify/query vport 
state")

from the net-next tree and commit:

   e5f6175c5b66 ("net/mlx5_core: Break down the vport mac address query 
function")

from the rdma tree and maybe some others.

I have no hope of fixing this stuff up, so I have dropped the rdma tree
again for today.  There is similar functionality being introduced in
both trees ... please sort this mess out ...


Hi Stephen,

Saeed/Matan and Co are working here on a fix which would solve the 
conflict.


We have the basic patch and people will be testing it later/tomorrow.

Could you please advice how would it be possible to provide you the
git rerere output -- the pre-image and post-images files from .git/rr-cache?

Or.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: linux-next: manual merge of the rdma tree with the net-next tree

[Or Gerlitz]

On 1/5/2016 3:51 AM, Stephen Rothwell wrote:

Hi Doug,

Today's linux-next merge of the rdma tree got conflicts in:

   drivers/net/ethernet/mellanox/mlx5/core/vport.c
   include/linux/mlx5/mlx5_ifc.h
   include/linux/mlx5/vport.h

between commits:

   e1d7d349c69d ("net/mlx5: Update access functions to Query/Modify vport MAC 
address")
   e75465148b7d ("net/mlx5: Introduce access functions to modify/query vport 
state")

from the net-next tree and commit:

   e5f6175c5b66 ("net/mlx5_core: Break down the vport mac address query 
function")

from the rdma tree and maybe some others.

I have no hope of fixing this stuff up, so I have dropped the rdma tree
again for today.  There is similar functionality being introduced in
both trees ... please sort this mess out ...


Hi Stephen,

Saeed/Matan and Co are working here on a fix which would solve the 
conflict.


We have the basic patch and people will be testing it later/tomorrow.

Could you please advice how would it be possible to provide you the
git rerere output -- the pre-image and post-images files from .git/rr-cache?

Or.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net/mlx4_core: fix handling return value of mlx4_slave_convert_port

[Or Gerlitz]

On 12/14/2015 12:05 PM, Andrzej Hajda wrote:

The function can return negative values, so its result should
be assigned to signed variable.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/assign_signed_to_unsigned.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2046107



Please add here

Fixes: fc48866f7 ('net/mlx4: Adapt code for N-Port VF')



Signed-off-by: Andrzej Hajda 


otherwise, Looks good

Acked-by: Or Gerlitz 

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] net/mlx4_core: fix handling return value of mlx4_slave_convert_port

[Or Gerlitz]

On 12/14/2015 12:05 PM, Andrzej Hajda wrote:

The function can return negative values, so its result should
be assigned to signed variable.

The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/assign_signed_to_unsigned.cocci [1].

[1]: http://permalink.gmane.org/gmane.linux.kernel/2046107



Please add here

Fixes: fc48866f7 ('net/mlx4: Adapt code for N-Port VF')



Signed-off-by: Andrzej Hajda <a.ha...@samsung.com>


otherwise, Looks good

Acked-by: Or Gerlitz <ogerl...@mellanox.com>

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/9] IB/iser: Use a dedicated descriptor for login

[Or Gerlitz]

On 11/13/2015 3:46 PM, Christoph Hellwig wrote:

From: Sagi Grimberg

Makes better sense and we'll need it later with CQ abstraction.
iser switch login bufs to void


Sagi, few quick comments on this patch, please address for next version..

The 2nd sentence of the change-log needs better phrasing.

also multiple checkpatch hits on the patch, please fix

CHECK: Please don't use multiple blank lines
#26: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:329:

+

WARNING: __packed is preferred over __attribute__((packed))
#42: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:345:
+} __attribute__((packed));

CHECK: Please don't use multiple blank lines
#44: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:347:
+
+

CHECK: Alignment should match open parenthesis
#161: FILE: drivers/infiniband/ulp/iser/iser_initiator.c:209:
+   if (ib_dma_mapping_error(device->ib_device,
+   desc->req_dma))

CHECK: Alignment should match open parenthesis
#172: FILE: drivers/infiniband/ulp/iser/iser_initiator.c:220:
+   if (ib_dma_mapping_error(device->ib_device,
+   desc->rsp_dma))






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 9/9] IB/iser: Convert to CQ abstraction

[Or Gerlitz]

On 11/13/2015 3:46 PM, Christoph Hellwig wrote:

From: Sagi Grimberg


Care to sparse some text here to assist a reviewer and future bisections?!

I have asked multiple times to avoid empty change-logs for patches in 
this driver.




Signed-off-by: Sagi Grimberg
Signed-off-by: Christoph Hellwig
---
  drivers/infiniband/ulp/iser/iscsi_iser.h |  68 ---
  drivers/infiniband/ulp/iser/iser_initiator.c | 142 ++-
  drivers/infiniband/ulp/iser/iser_memory.c|  21 ++-
  drivers/infiniband/ulp/iser/iser_verbs.c | 258 ++-
  4 files changed, 209 insertions(+), 280 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/9] move blk_iopoll to limit and make it generally available

[Or Gerlitz]

On Sun, Nov 15, 2015 at 10:48 AM, Sagi Grimberg
 wrote:
> Or is correct,
>
> I have attempted to convert iser to use blk_iopoll in the past, however
> I've seen inconsistent performance and latency skews (comparing to
> tasklets iser is using today). This was manifested in IOPs test cases
> where I ran multiple threads with higher queue-depth and not in
> sanitized pure latency (QD=1) test cases. Unfortunately I didn't have
> the time to pick it up since.
>
> I do have every intention of testing it again with this. If it still
> exist we will need to find the root-cause of it before converting
> drivers to use it.

Good, this way (inconsistent performance and latency skews) or another
(all shines up) -- please
let us know your findings, best through commenting within V > 0 the
cover letter posts of this series
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/9] move blk_iopoll to limit and make it generally available

[Or Gerlitz]

On Sun, Nov 15, 2015 at 10:48 AM, Sagi Grimberg
 wrote:
> Or is correct,
>
> I have attempted to convert iser to use blk_iopoll in the past, however
> I've seen inconsistent performance and latency skews (comparing to
> tasklets iser is using today). This was manifested in IOPs test cases
> where I ran multiple threads with higher queue-depth and not in
> sanitized pure latency (QD=1) test cases. Unfortunately I didn't have
> the time to pick it up since.
>
> I do have every intention of testing it again with this. If it still
> exist we will need to find the root-cause of it before converting
> drivers to use it.

Good, this way (inconsistent performance and latency skews) or another
(all shines up) -- please
let us know your findings, best through commenting within V > 0 the
cover letter posts of this series
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 9/9] IB/iser: Convert to CQ abstraction

[Or Gerlitz]

On 11/13/2015 3:46 PM, Christoph Hellwig wrote:

From: Sagi Grimberg


Care to sparse some text here to assist a reviewer and future bisections?!

I have asked multiple times to avoid empty change-logs for patches in 
this driver.




Signed-off-by: Sagi Grimberg
Signed-off-by: Christoph Hellwig
---
  drivers/infiniband/ulp/iser/iscsi_iser.h |  68 ---
  drivers/infiniband/ulp/iser/iser_initiator.c | 142 ++-
  drivers/infiniband/ulp/iser/iser_memory.c|  21 ++-
  drivers/infiniband/ulp/iser/iser_verbs.c | 258 ++-
  4 files changed, 209 insertions(+), 280 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 7/9] IB/iser: Use a dedicated descriptor for login

[Or Gerlitz]

On 11/13/2015 3:46 PM, Christoph Hellwig wrote:

From: Sagi Grimberg

Makes better sense and we'll need it later with CQ abstraction.
iser switch login bufs to void


Sagi, few quick comments on this patch, please address for next version..

The 2nd sentence of the change-log needs better phrasing.

also multiple checkpatch hits on the patch, please fix

CHECK: Please don't use multiple blank lines
#26: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:329:

+

WARNING: __packed is preferred over __attribute__((packed))
#42: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:345:
+} __attribute__((packed));

CHECK: Please don't use multiple blank lines
#44: FILE: drivers/infiniband/ulp/iser/iscsi_iser.h:347:
+
+

CHECK: Alignment should match open parenthesis
#161: FILE: drivers/infiniband/ulp/iser/iser_initiator.c:209:
+   if (ib_dma_mapping_error(device->ib_device,
+   desc->req_dma))

CHECK: Alignment should match open parenthesis
#172: FILE: drivers/infiniband/ulp/iser/iser_initiator.c:220:
+   if (ib_dma_mapping_error(device->ib_device,
+   desc->rsp_dma))






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/9] move blk_iopoll to limit and make it generally available

[Or Gerlitz]

On Fri, Nov 13, 2015 at 3:46 PM, Christoph Hellwig  wrote:
> The new name is irq_poll as iopoll is already taken.  Better suggestions
> welcome.

Sagi (or Christoph if you can address that),

@ some pointer over the last 18 months there was a port done at
mellanox for iser to use blk-iopoll and AFAIR it didn't work well or
didn't work at all. Can you tell now what was the problem and how did
you address it at your generalization?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/9] move blk_iopoll to limit and make it generally available

[Or Gerlitz]

On Fri, Nov 13, 2015 at 3:46 PM, Christoph Hellwig  wrote:
> The new name is irq_poll as iopoll is already taken.  Better suggestions
> welcome.

Sagi (or Christoph if you can address that),

@ some pointer over the last 18 months there was a port done at
mellanox for iser to use blk-iopoll and AFAIR it didn't work well or
didn't work at all. Can you tell now what was the problem and how did
you address it at your generalization?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC

[Or Gerlitz]

On Wed, Oct 21, 2015 at 10:20 PM, Alex Williamson
 wrote:

> This is why the typical VF agnostic approach here is to using bonding
> and fail over to a emulated device during migration, so performance
> suffers, but downtime is something acceptable.

bonding in the VM isn't a zero touch solution, right? is it really acceptable?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC

[Or Gerlitz]

On Wed, Oct 21, 2015 at 10:20 PM, Alex Williamson
 wrote:

> This is why the typical VF agnostic approach here is to using bonding
> and fail over to a emulated device during migration, so performance
> suffers, but downtime is something acceptable.

bonding in the VM isn't a zero touch solution, right? is it really acceptable?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC

[Or Gerlitz]

On Wed, Oct 21, 2015 at 7:37 PM, Lan Tianyu  wrote:
> This patchset is to propose a new solution to add live migration support
> for 82599 SRIOV network card.

> In our solution, we prefer to put all device specific operation into VF and
> PF driver and make code in the Qemu more general.

[...]

> Service down time test
> So far, we tested migration between two laptops with 82599 nic which
> are connected to a gigabit switch. Ping VF in the 0.001s interval
> during migration on the host of source side. It service down
> time is about 180ms.

So... what would you expect service down wise for the following
solution which is zero touch and I think should work for any VF
driver:

on host A: unplug the VM and conduct live migration to host B ala the
no-SRIOV case.

on host B:

when the VM "gets back to live", probe a VF there with the same assigned mac

next, udev on the VM will call the VF driver to create netdev instance

DHCP client would run to get the same IP address

+ under config directive (or from Qemu) send Gratuitous ARP to notify
the switch/es on the new location for that mac.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC Patch 00/12] IXGBE: Add live migration support for SRIOV NIC

[Or Gerlitz]

On Wed, Oct 21, 2015 at 7:37 PM, Lan Tianyu  wrote:
> This patchset is to propose a new solution to add live migration support
> for 82599 SRIOV network card.

> In our solution, we prefer to put all device specific operation into VF and
> PF driver and make code in the Qemu more general.

[...]

> Service down time test
> So far, we tested migration between two laptops with 82599 nic which
> are connected to a gigabit switch. Ping VF in the 0.001s interval
> during migration on the host of source side. It service down
> time is about 180ms.

So... what would you expect service down wise for the following
solution which is zero touch and I think should work for any VF
driver:

on host A: unplug the VM and conduct live migration to host B ala the
no-SRIOV case.

on host B:

when the VM "gets back to live", probe a VF there with the same assigned mac

next, udev on the VM will call the VF driver to create netdev instance

DHCP client would run to get the same IP address

+ under config directive (or from Qemu) send Gratuitous ARP to notify
the switch/es on the new location for that mac.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] fix return value error

[Or Gerlitz]

On 10/14/2015 2:59 PM, Leon Romanovsky wrote:

On Wed, Oct 14, 2015 at 11:17 AM, Heloise NH  wrote:

>Signed-off-by: Heloise NH

The patch is a correct one, however can you update the subject and
description to be more informative?
Please add that new_inode() function can fail for allocation only.


>---
>  drivers/infiniband/hw/ipath/ipath_fs.c | 2 +-
>  drivers/infiniband/hw/qib/qib_fs.c | 2 +-


NO... the ipath driver has moved to staging as part of its EOL process, 
it's now under drivers/staging/rdma/ipath

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] fix return value error

[Or Gerlitz]

On 10/14/2015 2:59 PM, Leon Romanovsky wrote:

On Wed, Oct 14, 2015 at 11:17 AM, Heloise NH  wrote:

>Signed-off-by: Heloise NH

The patch is a correct one, however can you update the subject and
description to be more informative?
Please add that new_inode() function can fail for allocation only.


>---
>  drivers/infiniband/hw/ipath/ipath_fs.c | 2 +-
>  drivers/infiniband/hw/qib/qib_fs.c | 2 +-


NO... the ipath driver has moved to staging as part of its EOL process, 
it's now under drivers/staging/rdma/ipath

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] IB/mad: remove obsolete snoop interface

[Or Gerlitz]

On Wed, Sep 30, 2015 at 9:01 AM,   wrote:

> This interface has no current users and is obsolete.

mmm, how does Sean's madeye util is working?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: rdma: Fix braces around if/else

[Or Gerlitz]

On 10/3/2015 11:55 PM, Martin Kletzander wrote:

Get rid of all ELSE_AFTER_BRACE type errors reported by checkpatch.pl.


Hi Greg,

Is there a way to signal people/tools that a certain driver parks in 
staging on their way **out** of the kernel
and not the other way around? I guess you (nor Doug) don't want to spend 
time on fixing such drivers, right?


Or.



Signed-off-by: Martin Kletzander 
---
There is one warning reported in this patch though.  That's because of
the multiline string and it's pre-existing.  Feel free to let me know
if that should be fixed too, I'd also remove the pointless '#' then.
On the other hand, it'll create more-than-80-columns long line.

  drivers/staging/rdma/ipath/ipath_driver.c| 19 ---
  drivers/staging/rdma/ipath/ipath_file_ops.c  | 12 ++--
  drivers/staging/rdma/ipath/ipath_iba6110.c   |  7 +++
  drivers/staging/rdma/ipath/ipath_init_chip.c | 10 +-
  drivers/staging/rdma/ipath/ipath_intr.c  |  7 +++
  drivers/staging/rdma/ipath/ipath_sysfs.c |  7 +++
  drivers/staging/rdma/ipath/ipath_verbs.c |  4 ++--
  7 files changed, 30 insertions(+), 36 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] staging: rdma: Fix braces around if/else

[Or Gerlitz]

On 10/3/2015 11:55 PM, Martin Kletzander wrote:

Get rid of all ELSE_AFTER_BRACE type errors reported by checkpatch.pl.


Hi Greg,

Is there a way to signal people/tools that a certain driver parks in 
staging on their way **out** of the kernel
and not the other way around? I guess you (nor Doug) don't want to spend 
time on fixing such drivers, right?


Or.



Signed-off-by: Martin Kletzander 
---
There is one warning reported in this patch though.  That's because of
the multiline string and it's pre-existing.  Feel free to let me know
if that should be fixed too, I'd also remove the pointless '#' then.
On the other hand, it'll create more-than-80-columns long line.

  drivers/staging/rdma/ipath/ipath_driver.c| 19 ---
  drivers/staging/rdma/ipath/ipath_file_ops.c  | 12 ++--
  drivers/staging/rdma/ipath/ipath_iba6110.c   |  7 +++
  drivers/staging/rdma/ipath/ipath_init_chip.c | 10 +-
  drivers/staging/rdma/ipath/ipath_intr.c  |  7 +++
  drivers/staging/rdma/ipath/ipath_sysfs.c |  7 +++
  drivers/staging/rdma/ipath/ipath_verbs.c |  4 ++--
  7 files changed, 30 insertions(+), 36 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH RFC] IB/mad: remove obsolete snoop interface

[Or Gerlitz]

On Wed, Sep 30, 2015 at 9:01 AM,   wrote:

> This interface has no current users and is obsolete.

mmm, how does Sean's madeye util is working?

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] infiniband:mlx4:Fix assumation that ib_get_cached_pkey runs successfully in build_mlx_header

[Or Gerlitz]

On 9/10/2015 2:04 AM, Nicholas Krause wrote:

This fixes a incorrect assumation that ib_get_cached_pkey always runs
successfully in the function build_mlx_header by checking if the calls
to this particular function return the error code, -EINVAL in order to
signal they failed to grap the public key for the device structure pointer
passed to this function and if so return immediately to the caller of
mlx_header with the error code -EINVAL to signal a error has occurred
when calling this particular function that the caller must handle in
its own intended error paths.


I am totally breathless, put 2-3 periods somewhere along this crazingly 
long sentence.


Does this fixes some specific commit? if yes, please add Fixes: tag @ 
before your S.O.B linewith the offending commit short ID (--abbrev=12)


run speller (e.g assumation  --> assumption), use IB/mlx4: prefix, add 
space between the ":" to the 1st word of the title, e.g:


IB/mlx4: Fix assumption that ib_get_cached_pkey runs successfully in 
build_mlx_header


Or.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] infiniband:mlx4:Fix assumation that ib_get_cached_pkey runs successfully in build_mlx_header

[Or Gerlitz]

On 9/10/2015 2:04 AM, Nicholas Krause wrote:

This fixes a incorrect assumation that ib_get_cached_pkey always runs
successfully in the function build_mlx_header by checking if the calls
to this particular function return the error code, -EINVAL in order to
signal they failed to grap the public key for the device structure pointer
passed to this function and if so return immediately to the caller of
mlx_header with the error code -EINVAL to signal a error has occurred
when calling this particular function that the caller must handle in
its own intended error paths.


I am totally breathless, put 2-3 periods somewhere along this crazingly 
long sentence.


Does this fixes some specific commit? if yes, please add Fixes: tag @ 
before your S.O.B linewith the offending commit short ID (--abbrev=12)


run speller (e.g assumation  --> assumption), use IB/mlx4: prefix, add 
space between the ":" to the 1st word of the title, e.g:


IB/mlx4: Fix assumption that ib_get_cached_pkey runs successfully in 
build_mlx_header


Or.



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/RFT PATCH v3 0/4] KVM: x86: full virtualization of guest MTRR

[Or Gerlitz]

On 7/8/2015 6:18 PM, Paolo Bonzini wrote:

This part of the MTRR patches was dropped by Xiao.  Bring SVM on feature
parity with VMX, and then do guest MTRR virtualization for both VMX and SVM.

The IPAT bit of VMX extended page tables is emulated by mangling the guest
PAT value.

I do not have any AMD machines that support an IOMMU, so I would like
some help testing these patches.  Thanks,




Hi Paolo,

We (finally) have results showing that the patches work well and provide 
benefit.


For getting better latency with ConnectX RDMA devices, we write send 
descriptors
to a write-combining (WC) mapped buffer instead of ringing a doorbell 
and having
the HW fetch the descriptor from system memory. In the mlx4 jargon, this 
optimization

is called Blue-Flame (BF).

To test the patches, we booted two hosts with 4.2-rc, patched with
this series, over which a legacy (RHEL 6.x) guests arerunning.

Under SRIOV, the mlx4 VF driver queries the mlx4 PF driver on the
host if BF is availablefor them to use.

We did two runs:

In [1] the guest VF driver was told by the host PF driver
that BF isn't supportedand hence they didn't use WC.

In [2] the VFs were told by the host PF driver that BF is supported
and hence used WC.

The results are provided in micro-seconds and account for half RTT of
nativeRDMA latency test, the WC advantage is notable, so +1for this series!

Or.


[1] guests not-using Blue-Flame / Write-Combining

root@host-194-168-80-68 ~]# ib_send_lat -a
 #bytes #iterationst_min[usec]t_max[usec] t_typical[usec]
 2   1000  1.13   11.531.16
 4   1000  1.13   6.33 1.16
 8   1000  1.13   5.17 1.17
 16  1000  1.14   4.37 1.17
 32  1000  1.15   5.01 1.18
 64  1000  1.19   7.96 1.22
 128 1000  1.28   5.44 1.31
 256 1000  1.62   6.90 1.65
 512 1000  1.78   5.65 1.82


[2] guests using Blue-Flame when the host allows Write-Combining mapping 
by VMs


root@host-194-168-80-68 ~]# ib_send_lat -a
#bytes #iterationst_min[usec]t_max[usec] t_typical[usec]
 2   1000  0.86   16.970.89
 4   1000  0.87   4.81 0.90
 8   1000  0.87   4.89 0.90
 16  1000  0.87   6.46 0.90
 32  1000  0.88   4.22 0.91
 64  1000  0.94   4.03 0.97
 128 1000  1.03   6.50 1.06
 256 1000  1.36   7.71 1.39
 512 1000  1.49   5.92 1.52



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RFC/RFT PATCH v3 0/4] KVM: x86: full virtualization of guest MTRR

[Or Gerlitz]

On 7/8/2015 6:18 PM, Paolo Bonzini wrote:

This part of the MTRR patches was dropped by Xiao.  Bring SVM on feature
parity with VMX, and then do guest MTRR virtualization for both VMX and SVM.

The IPAT bit of VMX extended page tables is emulated by mangling the guest
PAT value.

I do not have any AMD machines that support an IOMMU, so I would like
some help testing these patches.  Thanks,




Hi Paolo,

We (finally) have results showing that the patches work well and provide 
benefit.


For getting better latency with ConnectX RDMA devices, we write send 
descriptors
to a write-combining (WC) mapped buffer instead of ringing a doorbell 
and having
the HW fetch the descriptor from system memory. In the mlx4 jargon, this 
optimization

is called Blue-Flame (BF).

To test the patches, we booted two hosts with 4.2-rc, patched with
this series, over which a legacy (RHEL 6.x) guests arerunning.

Under SRIOV, the mlx4 VF driver queries the mlx4 PF driver on the
host if BF is availablefor them to use.

We did two runs:

In [1] the guest VF driver was told by the host PF driver
that BF isn't supportedand hence they didn't use WC.

In [2] the VFs were told by the host PF driver that BF is supported
and hence used WC.

The results are provided in micro-seconds and account for half RTT of
nativeRDMA latency test, the WC advantage is notable, so +1for this series!

Or.


[1] guests not-using Blue-Flame / Write-Combining

root@host-194-168-80-68 ~]# ib_send_lat -a
 #bytes #iterationst_min[usec]t_max[usec] t_typical[usec]
 2   1000  1.13   11.531.16
 4   1000  1.13   6.33 1.16
 8   1000  1.13   5.17 1.17
 16  1000  1.14   4.37 1.17
 32  1000  1.15   5.01 1.18
 64  1000  1.19   7.96 1.22
 128 1000  1.28   5.44 1.31
 256 1000  1.62   6.90 1.65
 512 1000  1.78   5.65 1.82


[2] guests using Blue-Flame when the host allows Write-Combining mapping 
by VMs


root@host-194-168-80-68 ~]# ib_send_lat -a
#bytes #iterationst_min[usec]t_max[usec] t_typical[usec]
 2   1000  0.86   16.970.89
 4   1000  0.87   4.81 0.90
 8   1000  0.87   4.89 0.90
 16  1000  0.87   6.46 0.90
 32  1000  0.88   4.22 0.91
 64  1000  0.94   4.03 0.97
 128 1000  1.03   6.50 1.06
 256 1000  1.36   7.71 1.39
 512 1000  1.49   5.92 1.52



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On 7/14/2015 11:28 PM, Alex Thorlton wrote:


We see the same exact messages on 4.1-rc8.




does this solves the problem?


diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index ad31e47..c8ae3b9 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -45,7 +45,7 @@
 #include 

 #define MAX_MSIX_P_PORT17
-#define MAX_MSIX   64
+#define MAX_MSIX   1024
 #define MIN_MSIX_P_PORT5
 #define MLX4_IS_LEGACY_EQ_MODE(dev_cap) ((dev_cap).num_comp_vectors < \
(dev_cap).num_ports * MIN_MSIX_P_PORT)
--

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On 7/14/2015 11:28 PM, Alex Thorlton wrote:


We see the same exact messages on 4.1-rc8.




does this solves the problem?


diff --git a/include/linux/mlx4/device.h b/include/linux/mlx4/device.h
index ad31e47..c8ae3b9 100644
--- a/include/linux/mlx4/device.h
+++ b/include/linux/mlx4/device.h
@@ -45,7 +45,7 @@
 #include linux/timecounter.h

 #define MAX_MSIX_P_PORT17
-#define MAX_MSIX   64
+#define MAX_MSIX   1024
 #define MIN_MSIX_P_PORT5
 #define MLX4_IS_LEGACY_EQ_MODE(dev_cap) ((dev_cap).num_comp_vectors  \
(dev_cap).num_ports * MIN_MSIX_P_PORT)
--

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On Tue, Jul 14, 2015 at 9:48 PM, Alex Thorlton  wrote:
> On Tue, Jul 14, 2015 at 01:22:34PM -0500, andrew banman wrote:
>> On Sat, Jul 11, 2015 at 11:20:19PM +0300, Or Gerlitz wrote:
>> > On Fri, Jul 10, 2015 at 10:15 PM, andrew banman  wrote:
>> > > I'm seeing a large number of allocation errors originating from the 
>> > > Mellanox IB
>> > > driver when booting the 4.2-rc1 kernel on a 4096cpu 32TB memory system:
>> >
>> > Just to make sure, mlx4 works fine on this small (...) system with 4.1
>> > and 4.2-rc1 breaks, or 4.2-rc1 is the 1st time you're trying that
>> > config?
>>
>> I'll let Alex comment on that, he did some testing on that.
>
> I started seeing this on a 4.1-rc8 kernel, so it's been around for a
> little while.  It may have been around before 4.1-rc8, but I haven't run
> any kernels older than that on the big machine for some time.

To make sure I am correctly following, on 4.1-rc8  you also see
something, right? are these the same messages or different ones? if
the latter send to us.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On Tue, Jul 14, 2015 at 9:48 PM, Alex Thorlton athorl...@sgi.com wrote:
 On Tue, Jul 14, 2015 at 01:22:34PM -0500, andrew banman wrote:
 On Sat, Jul 11, 2015 at 11:20:19PM +0300, Or Gerlitz wrote:
  On Fri, Jul 10, 2015 at 10:15 PM, andrew banman aban...@sgi.com wrote:
   I'm seeing a large number of allocation errors originating from the 
   Mellanox IB
   driver when booting the 4.2-rc1 kernel on a 4096cpu 32TB memory system:
 
  Just to make sure, mlx4 works fine on this small (...) system with 4.1
  and 4.2-rc1 breaks, or 4.2-rc1 is the 1st time you're trying that
  config?

 I'll let Alex comment on that, he did some testing on that.

 I started seeing this on a 4.1-rc8 kernel, so it's been around for a
 little while.  It may have been around before 4.1-rc8, but I haven't run
 any kernels older than that on the big machine for some time.

To make sure I am correctly following, on 4.1-rc8  you also see
something, right? are these the same messages or different ones? if
the latter send to us.

Or.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On Fri, Jul 10, 2015 at 10:15 PM, andrew banman  wrote:
> I'm seeing a large number of allocation errors originating from the Mellanox 
> IB
> driver when booting the 4.2-rc1 kernel on a 4096cpu 32TB memory system:

Just to make sure, mlx4 works fine on this small (...) system with 4.1
and 4.2-rc1 breaks, or 4.2-rc1 is the 1st time you're trying that
config?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [BUG] mellanox IB driver fails to load on large config

[Or Gerlitz]

On Fri, Jul 10, 2015 at 10:15 PM, andrew banman aban...@sgi.com wrote:
 I'm seeing a large number of allocation errors originating from the Mellanox 
 IB
 driver when booting the 4.2-rc1 kernel on a 4096cpu 32TB memory system:

Just to make sure, mlx4 works fine on this small (...) system with 4.1
and 4.2-rc1 breaks, or 4.2-rc1 is the 1st time you're trying that
config?
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mlx4: "failed to allocate default counter port 1"

[Or Gerlitz]

On Tue, Jun 30, 2015 at 1:45 PM, Sebastian Ott
 wrote:
> after the latest mellanox update the mlx4 driver fails to probe a VF:
> [   88.909562] mlx4_core :00:00.0: mlx4_allocate_default_counters: failed 
> to allocate default counter port 1 err -22
> [   88.909564] mlx4_core :00:00.0: Failed to allocate default counters, 
> aborting
> [   88.961735] mlx4_core: probe of :00:00.0 failed with error -22
>
> PFs still work. See below for more dmesg output - I also added a line of
> debug output...maybe this helps.

Can you please send your "lspci | grep nox" listing? also what
Firmware version you have there? e.g when you probe the PF with
mlx4_core debug_level=1 can you sens us the lines that follow the PF
probe, e.g as here + dump of all caps as you did for the VF

  952.367911] mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014)
[  952.374606] mlx4_core: Initializing :06:00.0
[  953.384332] mlx4_core :06:00.0: FW version 2.34.5000 (cmd intf
rev 3), max commands 16
[...]

Also send us the output of "dmesg | grep -i counter" after such verbose load.

thanks,

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: mlx4: failed to allocate default counter port 1

[Or Gerlitz]

On Tue, Jun 30, 2015 at 1:45 PM, Sebastian Ott
seb...@linux.vnet.ibm.com wrote:
 after the latest mellanox update the mlx4 driver fails to probe a VF:
 [   88.909562] mlx4_core :00:00.0: mlx4_allocate_default_counters: failed 
 to allocate default counter port 1 err -22
 [   88.909564] mlx4_core :00:00.0: Failed to allocate default counters, 
 aborting
 [   88.961735] mlx4_core: probe of :00:00.0 failed with error -22

 PFs still work. See below for more dmesg output - I also added a line of
 debug output...maybe this helps.

Can you please send your lspci | grep nox listing? also what
Firmware version you have there? e.g when you probe the PF with
mlx4_core debug_level=1 can you sens us the lines that follow the PF
probe, e.g as here + dump of all caps as you did for the VF

  952.367911] mlx4_core: Mellanox ConnectX core driver v2.2-1 (Feb, 2014)
[  952.374606] mlx4_core: Initializing :06:00.0
[  953.384332] mlx4_core :06:00.0: FW version 2.34.5000 (cmd intf
rev 3), max commands 16
[...]

Also send us the output of dmesg | grep -i counter after such verbose load.

thanks,

Or.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [GIT] Networking

[Or Gerlitz]

On Thu, Jun 25, 2015 at 2:38 AM, Linus Torvalds
 wrote:
>
> On Wed, Jun 24, 2015 at 6:39 AM, David Miller  wrote:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git master
>
> Just going through the conflicts, I see commit 7193a141eb74 ("IB/mlx4:
> Set VF to read from QP counters"), and wonder...
>
> Is that code really supposed to fall through to the
> infiniband-over-ethernet case when the link layer is
> IB_LINK_LAYER_INFINIBAND but it's a slave?
>
> The commit message is not in the least helpful.
>

And this is a bug indeed.

Under IB links, we should use the the infiniband-over-ethernet flow only for one
specific case (reading link performance counters by SRIOV VFs) and nothing else.

I sent a fix, https://patchwork.kernel.org/patch/6675921/

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/