[PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Thomas Falcon]

The function rtas_flash_firmware passes the address of a data structure,
flash_block_list, when making the update-flash-64-and-reboot rtas call.
While the endianness of the address is handled correctly, the endianness
of the data is not.  This patch ensures that the data in flash_block_list
is big endian when passed to rtas on little endian hosts.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/kernel/rtas_flash.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 658e89d..db2b482 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -611,17 +611,19 @@ static void rtas_flash_firmware(int reboot_type)
for (f = flist; f; f = next) {
/* Translate data addrs to absolute */
for (i = 0; i  f-num_blocks; i++) {
-   f-blocks[i].data = (char *)__pa(f-blocks[i].data);
+   f-blocks[i].data = (char 
*)cpu_to_be64(__pa(f-blocks[i].data));
image_size += f-blocks[i].length;
+   f-blocks[i].length = cpu_to_be64(f-blocks[i].length);
}
next = f-next;
/* Don't translate NULL pointer for last entry */
if (f-next)
-   f-next = (struct flash_block_list *)__pa(f-next);
+   f-next = (struct flash_block_list 
*)cpu_to_be64(__pa(f-next));
else
f-next = NULL;
/* make num_blocks into the version/length field */
f-num_blocks = (FLASH_BLOCK_LIST_VERSION  56) | 
((f-num_blocks+1)*16);
+   f-num_blocks = cpu_to_be64(f-num_blocks);
}
 
printk(KERN_ALERT FLASH: flash image is %ld bytes\n, image_size);
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] powerpc: fixing endianness of flash_block_list in rtas_flash

[Thomas Falcon]

On 08/01/2014 04:32 AM, Vasant Hegde wrote:

On 07/25/2014 11:17 PM, Thomas Falcon wrote:

The function rtas_flash_firmware passes the address of a data structure,
flash_block_list, when making the update-flash-64-and-reboot rtas call.
While the endianness of the address is handled correctly, the endianness
of the data is not.  This patch ensures that the data in 
flash_block_list

is big endian when passed to rtas on little endian hosts.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
  arch/powerpc/kernel/rtas_flash.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)



Tom,

In validate_flash rtas call returns  update_results in BE.. I think we 
need below changes as well.


Rest looks good.

diff --git a/arch/powerpc/kernel/rtas_flash.c 
b/arch/powerpc/kernel/rtas_flash.c

index db2b482..1eae0d8 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -449,7 +449,7 @@ error:
 static void validate_flash(struct rtas_validate_flash_t *args_buf)
 {
int token = rtas_token(ibm,validate-flash-image);
-   int update_results;
+   __be32 update_results;
s32 rc;

rc = 0;
@@ -463,7 +463,7 @@ static void validate_flash(struct 
rtas_validate_flash_t *args_buf)

} while (rtas_busy_delay(rc));

args_buf-status = rc;
-   args_buf-update_results = update_results;
+   args_buf-update_results = be32_to_cpu(update_results);
 }

I do not think this conversion is needed.  Any integers returned are 
converted to cpu endian in the rtas_call function.


tom


-Vasant



--

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/pseries/hvcserver: Fix endian issue in hvcs_get_partner_info

[Thomas Falcon]

A buffer returned by H_VTERM_PARTNER_INFO contains device information
in big endian format, causing problems for little endian architectures.
This patch ensures that they are in cpu endian.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/hvcserver.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hvcserver.c 
b/arch/powerpc/platforms/pseries/hvcserver.c
index 4557e91..eedb645 100644
--- a/arch/powerpc/platforms/pseries/hvcserver.c
+++ b/arch/powerpc/platforms/pseries/hvcserver.c
@@ -163,8 +163,8 @@ int hvcs_get_partner_info(uint32_t unit_address, struct 
list_head *head,
return retval;
}
 
-   last_p_partition_ID = pi_buff[0];
-   last_p_unit_address = pi_buff[1];
+   last_p_partition_ID = be64_to_cpu(pi_buff[0]);
+   last_p_unit_address = be64_to_cpu(pi_buff[1]);
 
/* This indicates that there are no further partners */
if (last_p_partition_ID == ~0UL
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] powerpc/pseries: Fix endian issues in memory hotplug

[Thomas Falcon]

Values acquired from Open Firmware are in 32-bit big endian format
and need to be handled on little endian architectures.  This patch
ensures values are in cpu endian when hotplugging memory.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 36 +
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
b/arch/powerpc/platforms/pseries/hotplug-memory.c
index c904583..17ee193 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -113,7 +113,7 @@ out:
 static int pseries_remove_mem_node(struct device_node *np)
 {
const char *type;
-   const unsigned int *regs;
+   const __be32 *regs;
unsigned long base;
unsigned int lmb_size;
int ret = -EINVAL;
@@ -132,8 +132,8 @@ static int pseries_remove_mem_node(struct device_node *np)
if (!regs)
return ret;
 
-   base = *(unsigned long *)regs;
-   lmb_size = regs[3];
+   base = be64_to_cpu(*(unsigned long *)regs);
+   lmb_size = be32_to_cpu(regs[3]);
 
pseries_remove_memblock(base, lmb_size);
return 0;
@@ -153,7 +153,7 @@ static inline int pseries_remove_mem_node(struct 
device_node *np)
 static int pseries_add_mem_node(struct device_node *np)
 {
const char *type;
-   const unsigned int *regs;
+   const __be32 *regs;
unsigned long base;
unsigned int lmb_size;
int ret = -EINVAL;
@@ -172,8 +172,8 @@ static int pseries_add_mem_node(struct device_node *np)
if (!regs)
return ret;
 
-   base = *(unsigned long *)regs;
-   lmb_size = regs[3];
+   base = be64_to_cpu(*(unsigned long *)regs);
+   lmb_size = be32_to_cpu(regs[3]);
 
/*
 * Update memory region to represent the memory add
@@ -187,14 +187,14 @@ static int pseries_update_drconf_memory(struct 
of_prop_reconfig *pr)
struct of_drconf_cell *new_drmem, *old_drmem;
unsigned long memblock_size;
u32 entries;
-   u32 *p;
+   __be32 *p;
int i, rc = -EINVAL;
 
memblock_size = pseries_memory_block_size();
if (!memblock_size)
return -EINVAL;
 
-   p = (u32 *) pr-old_prop-value;
+   p = (__be32 *) pr-old_prop-value;
if (!p)
return -EINVAL;
 
@@ -203,28 +203,30 @@ static int pseries_update_drconf_memory(struct 
of_prop_reconfig *pr)
 * entries. Get the niumber of entries and skip to the array of
 * of_drconf_cell's.
 */
-   entries = *p++;
+   entries = be32_to_cpu(*p++);
old_drmem = (struct of_drconf_cell *)p;
 
-   p = (u32 *)pr-prop-value;
+   p = (__be32 *)pr-prop-value;
p++;
new_drmem = (struct of_drconf_cell *)p;
 
for (i = 0; i  entries; i++) {
-   if ((old_drmem[i].flags  DRCONF_MEM_ASSIGNED) 
-   (!(new_drmem[i].flags  DRCONF_MEM_ASSIGNED))) {
-   rc = pseries_remove_memblock(old_drmem[i].base_addr,
+   if ((be32_to_cpu(old_drmem[i].flags)  DRCONF_MEM_ASSIGNED) 
+   (!(be32_to_cpu(new_drmem[i].flags)  DRCONF_MEM_ASSIGNED))) 
{
+   rc = pseries_remove_memblock(
+   be64_to_cpu(old_drmem[i].base_addr),
 memblock_size);
break;
-   } else if ((!(old_drmem[i].flags  DRCONF_MEM_ASSIGNED)) 
-  (new_drmem[i].flags  DRCONF_MEM_ASSIGNED)) {
-   rc = memblock_add(old_drmem[i].base_addr,
+   } else if ((!(be32_to_cpu(old_drmem[i].flags) 
+   DRCONF_MEM_ASSIGNED)) 
+   (be32_to_cpu(new_drmem[i].flags) 
+   DRCONF_MEM_ASSIGNED)) {
+   rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
  memblock_size);
rc = (rc  0) ? -EINVAL : 0;
break;
}
}
-
return rc;
 }
 
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] pseries: Fix endianness in cpu hotplug and hotremove

[Thomas Falcon]

This patch attempts to ensure that all values are in the proper
endianness format when both hotadding and hotremoving cpus.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/dlpar.c   | 56 ++--
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 20 +-
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a2450b8..c1d7e40 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -24,11 +24,11 @@
 #include asm/rtas.h
 
 struct cc_workarea {
-   u32 drc_index;
-   u32 zero;
-   u32 name_offset;
-   u32 prop_length;
-   u32 prop_offset;
+   __be32  drc_index;
+   __be32  zero;
+   __be32  name_offset;
+   __be32  prop_length;
+   __be32  prop_offset;
 };
 
 void dlpar_free_cc_property(struct property *prop)
@@ -48,11 +48,11 @@ static struct property *dlpar_parse_cc_property(struct 
cc_workarea *ccwa)
if (!prop)
return NULL;
 
-   name = (char *)ccwa + ccwa-name_offset;
+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
prop-name = kstrdup(name, GFP_KERNEL);
 
-   prop-length = ccwa-prop_length;
-   value = (char *)ccwa + ccwa-prop_offset;
+   prop-length = be32_to_cpu(ccwa-prop_length);
+   value = (char *)ccwa + be32_to_cpu(ccwa-prop_offset);
prop-value = kmemdup(value, prop-length, GFP_KERNEL);
if (!prop-value) {
dlpar_free_cc_property(prop);
@@ -78,7 +78,7 @@ static struct device_node *dlpar_parse_cc_node(struct 
cc_workarea *ccwa,
if (!dn)
return NULL;
 
-   name = (char *)ccwa + ccwa-name_offset;
+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
dn-full_name = kasprintf(GFP_KERNEL, %s/%s, path, name);
if (!dn-full_name) {
kfree(dn);
@@ -148,7 +148,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index,
return NULL;
 
ccwa = (struct cc_workarea *)data_buf[0];
-   ccwa-drc_index = drc_index;
+   ccwa-drc_index = cpu_to_be32(drc_index);
ccwa-zero = 0;
 
do {
@@ -363,10 +363,10 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv_be;
 
-   intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
-   if (!intserv)
+   intserv_be = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
+   if (!intserv_be)
return -EINVAL;
 
nthreads = len / sizeof(u32);
@@ -374,7 +374,7 @@ static int dlpar_online_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv_be[i]))
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +388,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to online 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv_be[i]));
}
cpu_maps_update_done();
 
@@ -442,18 +442,17 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv_be;
 
-   intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
-   if (!intserv)
+   intserv_be = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
+   if (!intserv_be)
return -EINVAL;
 
nthreads = len / sizeof(u32);
-
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv_be[i]))
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -469,20 +468,19 @@ static int dlpar_offline_cpu(struct device_node *dn)
break;
 
}
-
/*
 * The cpu is in CPU_STATE_INACTIVE.
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE

Re: [PATCH] pseries: Fix endianness in cpu hotplug and hotremove

[Thomas Falcon]

I guess we were both working on it independently.  I had made the 
changes to hotplug a cpu a few weeks ago, but was blocked on removing a 
cpu.  Last week I realized what was blocking me and fixed cpu removal, 
so I sent the patch along.  Since Bharata has already submitted a patch 
that will handle hotplugging, I'll resubmit this with only the changes 
needed to remove a cpu.


Tom

On 09/08/2014 09:07 AM, Nathan Fontenot wrote:

It looks like you have a lot of the same changes as the patch Bharata
sent out last week. Including the one issue I saw in Bharata's patch
below.

On 09/05/2014 02:09 PM, Thomas Falcon wrote:

This patch attempts to ensure that all values are in the proper
endianness format when both hotadding and hotremoving cpus.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
  arch/powerpc/platforms/pseries/dlpar.c   | 56 ++--
  arch/powerpc/platforms/pseries/hotplug-cpu.c | 20 +-
  2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a2450b8..c1d7e40 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -24,11 +24,11 @@
  #include asm/rtas.h
  
  struct cc_workarea {

-   u32 drc_index;
-   u32 zero;
-   u32 name_offset;
-   u32 prop_length;
-   u32 prop_offset;
+   __be32  drc_index;
+   __be32  zero;
+   __be32  name_offset;
+   __be32  prop_length;
+   __be32  prop_offset;
  };
  
  void dlpar_free_cc_property(struct property *prop)

@@ -48,11 +48,11 @@ static struct property *dlpar_parse_cc_property(struct 
cc_workarea *ccwa)
if (!prop)
return NULL;
  
-	name = (char *)ccwa + ccwa-name_offset;

+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
prop-name = kstrdup(name, GFP_KERNEL);
  
-	prop-length = ccwa-prop_length;

-   value = (char *)ccwa + ccwa-prop_offset;
+   prop-length = be32_to_cpu(ccwa-prop_length);
+   value = (char *)ccwa + be32_to_cpu(ccwa-prop_offset);
prop-value = kmemdup(value, prop-length, GFP_KERNEL);
if (!prop-value) {
dlpar_free_cc_property(prop);
@@ -78,7 +78,7 @@ static struct device_node *dlpar_parse_cc_node(struct 
cc_workarea *ccwa,
if (!dn)
return NULL;
  
-	name = (char *)ccwa + ccwa-name_offset;

+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
dn-full_name = kasprintf(GFP_KERNEL, %s/%s, path, name);
if (!dn-full_name) {
kfree(dn);
@@ -148,7 +148,7 @@ struct device_node *dlpar_configure_connector(u32 drc_index,
return NULL;
  
  	ccwa = (struct cc_workarea *)data_buf[0];

-   ccwa-drc_index = drc_index;
+   ccwa-drc_index = cpu_to_be32(drc_index);

This will break partition migration.

The drc index valued passed into dlpar_configure_connector() from the migration
path, pseries_devicetree_update(), is already in BE format.

-Nathan


ccwa-zero = 0;
  
  	do {

@@ -363,10 +363,10 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv_be;
  
-	intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);

-   if (!intserv)
+   intserv_be = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
+   if (!intserv_be)
return -EINVAL;
  
  	nthreads = len / sizeof(u32);

@@ -374,7 +374,7 @@ static int dlpar_online_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv_be[i]))
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +388,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to online 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv_be[i]));
}
cpu_maps_update_done();
  
@@ -442,18 +442,17 @@ static int dlpar_offline_cpu(struct device_node *dn)

int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv_be;
  
-	intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);

-   if (!intserv)
+   intserv_be = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
+   if (!intserv_be)
return -EINVAL;
  
  	nthreads = len / sizeof(u32

[PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

[Thomas Falcon]

When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index cd425dc..c5ecfdb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -475,14 +475,14 @@ static int dlpar_offline_cpu(struct device_node *dn)
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
-   BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+   BUG_ON(plpar_hcall_norets(H_PROD, 
be32_to_cpu(intserv[i]))
!= H_SUCCESS);
__cpu_die(cpu);
break;
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to offline 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 
@@ -494,7 +494,7 @@ out:
 static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 {
struct device_node *dn;
-   const u32 *drc_index;
+   const __be32 *drc_index;
int rc;
 
dn = of_find_node_by_path(buf);
@@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(be32_to_cpup(drc_index));
if (rc) {
of_node_put(dn);
return rc;
@@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
 
rc = dlpar_detach_node(dn);
if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(be32_to_cpup(drc_index));
return rc;
}
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 447f8c6..031762d 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -90,7 +90,7 @@ static void rtas_stop_self(void)
 {
static struct rtas_args args = {
.nargs = 0,
-   .nret = 1,
+   .nret = cpu_to_be32(1),
.rets = args.args[0],
};
 
@@ -312,7 +312,7 @@ static void pseries_remove_processor(struct device_node *np)
 {
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(np, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -323,7 +323,7 @@ static void pseries_remove_processor(struct device_node *np)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
BUG_ON(cpu_online(cpu));
set_cpu_present(cpu, false);
@@ -332,7 +332,7 @@ static void pseries_remove_processor(struct device_node *np)
}
if (cpu = nr_cpu_ids)
printk(KERN_WARNING Could not find cpu to remove 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 }
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/2] pseries: Fix endian issues in onlining cpu threads

[Thomas Falcon]

The ibm,ppc-interrupt-server#s property is in big endian format.
These values need to be converted when used by little endian
architectures.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 arch/powerpc/platforms/pseries/dlpar.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index c2806c8..cd425dc 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -363,7 +363,7 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -374,7 +374,7 @@ static int dlpar_online_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +388,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to online 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, 
be32_to_cpu(intserv[i]));
}
cpu_maps_update_done();
 
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal

[Thomas Falcon]

On 09/12/2014 03:53 AM, Michael Ellerman wrote:

On Wed, 2014-09-10 at 17:41 -0500, Thomas Falcon wrote:

When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
  arch/powerpc/platforms/pseries/dlpar.c   | 14 +++---
  arch/powerpc/platforms/pseries/hotplug-cpu.c |  8 
  2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index cd425dc..c5ecfdb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
  
  	intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);

if (!intserv)
@@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {

Can you please do the conversion once here for each value of i.

You can call the converted value thread ?


for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != 
be32_to_cpu(intserv[i]))
continue;

Rather than doing it for every cpu in the system for every value of i.

Not that performance is really an issue, but it's just ugly.

And obviously the other places that use it in the loop should use the converted
value.



@@ -494,7 +494,7 @@ out:
  static ssize_t dlpar_cpu_release(const char *buf, size_t count)
  {
struct device_node *dn;
-   const u32 *drc_index;
+   const __be32 *drc_index;
int rc;
  
  	dn = of_find_node_by_path(buf);

@@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}

Here again you should do the conversion once.

Better still use of_property_read_u32().


-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(be32_to_cpup(drc_index));
if (rc) {
of_node_put(dn);
return rc;
@@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
  
  	rc = dlpar_detach_node(dn);

if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(be32_to_cpup(drc_index));
return rc;
}

cheers


Thanks for the feedback.  Sending patches with your suggested changes.

tom





___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 1/2] pseries: Fix endian issues in onlining cpu threads

[Thomas Falcon]

The ibm,ppc-interrupt-server#s property is in big endian format.
These values need to be converted when used by little endian
architectures.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
Changes in v2:

 Followed suggestions from Michael Ellerman
   conversion of intserv values occur once
---
 arch/powerpc/platforms/pseries/dlpar.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index c2806c8..9e9f30b2 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -363,7 +363,8 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -373,8 +374,9 @@ static int dlpar_online_cpu(struct device_node *dn)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +390,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to online 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, thread);
}
cpu_maps_update_done();
 
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 2/2] pseries: Fix endian issues in cpu hot-removal

[Thomas Falcon]

When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
Changes in v2:

 Followed suggestions from Michael Ellerman:
   Conversion of intserv to cpu endian occurs once.
   Conversion of drc_index to cpu endian occurs once
   using of_property_read_u32.
---
 arch/powerpc/platforms/pseries/dlpar.c   | 20 +++-
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 10 ++
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index 9e9f30b2..343dfdf 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -444,7 +444,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -454,8 +455,9 @@ static int dlpar_offline_cpu(struct device_node *dn)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -477,14 +479,14 @@ static int dlpar_offline_cpu(struct device_node *dn)
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
-   BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+   BUG_ON(plpar_hcall_norets(H_PROD, thread)
!= H_SUCCESS);
__cpu_die(cpu);
break;
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to offline 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, thread);
}
cpu_maps_update_done();
 
@@ -496,15 +498,15 @@ out:
 static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 {
struct device_node *dn;
-   const u32 *drc_index;
+   const u32 drc_index;
int rc;
 
dn = of_find_node_by_path(buf);
if (!dn)
return -EINVAL;
 
-   drc_index = of_get_property(dn, ibm,my-drc-index, NULL);
-   if (!drc_index) {
+   rc = of_property_read_u32(dn, ibm,my-drc-index, drc_index);
+   if (rc) {
of_node_put(dn);
return -EINVAL;
}
@@ -515,7 +517,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(drc_index);
if (rc) {
of_node_put(dn);
return rc;
@@ -523,7 +525,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
 
rc = dlpar_detach_node(dn);
if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(drc_index);
return rc;
}
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 447f8c6..5c375f9 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -90,7 +90,7 @@ static void rtas_stop_self(void)
 {
static struct rtas_args args = {
.nargs = 0,
-   .nret = 1,
+   .nret = cpu_to_be32(1),
.rets = args.args[0],
};
 
@@ -312,7 +312,8 @@ static void pseries_remove_processor(struct device_node *np)
 {
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(np, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -322,8 +323,9 @@ static void pseries_remove_processor(struct device_node *np)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
BUG_ON(cpu_online(cpu));
set_cpu_present(cpu, false);
@@ -332,7 +334,7 @@ static void pseries_remove_processor(struct device_node *np

[PATCH 0/3] pseries: Make CPU hotplug and hotremove endian safe

[Thomas Falcon]

This patchset ensures that cpu hotplugging and hotremoval are 
compatible with both big and little endian architectures.  

Bharata B Rao (1):
  pseries: Make CPU hotplug path endian safe

Thomas Falcon (2):
  pseries: Fix endian issues in onlining cpu threads
  pseries: Fix endian issues in cpu hot-removal

 arch/powerpc/platforms/pseries/dlpar.c   | 50 +++-
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 14 
 arch/powerpc/platforms/pseries/pseries.h |  3 +-
 3 files changed, 37 insertions(+), 30 deletions(-)

-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 2/3] pseries: Fix endian issues in onlining cpu threads

[Thomas Falcon]

The ibm,ppc-interrupt-server#s property is in big endian format.
These values need to be converted when used by little endian
architectures.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
Changes in v2:

 Followed suggestions from Michael Ellerman
   conversion of intserv values occur once
---
 arch/powerpc/platforms/pseries/dlpar.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index 5acbe59..187e4eb 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -363,7 +363,8 @@ static int dlpar_online_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -373,8 +374,9 @@ static int dlpar_online_cpu(struct device_node *dn)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
BUG_ON(get_cpu_current_state(cpu)
!= CPU_STATE_OFFLINE);
@@ -388,7 +390,7 @@ static int dlpar_online_cpu(struct device_node *dn)
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to online 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, thread);
}
cpu_maps_update_done();
 
-- 
1.8.5.2

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v2 1/3] pseries: Make CPU hotplug path endian safe

[Thomas Falcon]

From: Bharata B Rao bhar...@linux.vnet.ibm.com

- ibm,rtas-configure-connector should treat the RTAS data as big endian.
- Treat ibm,ppc-interrupt-server#s as big-endian when setting
  smp_processor_id during hotplug.

Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---

Changes in v2:
- Don't convert drc_index to BE in dlpar_configure_connector() but instead
  convert in the caller dlpar_cpu_probe() so that migration path isn't
  affected.
- Mark members of cc_workarea struct as __be32 instead of u32 (Thomas)
- Based on top of Thomas Falcon's two patches.
  (http://patchwork.ozlabs.org/patch/388767/)

v1: http://patchwork.ozlabs.org/patch/386216/

 arch/powerpc/platforms/pseries/dlpar.c   | 22 +++---
 arch/powerpc/platforms/pseries/hotplug-cpu.c |  4 ++--
 arch/powerpc/platforms/pseries/pseries.h |  3 ++-
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index a2450b8..5acbe59 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -24,11 +24,11 @@
 #include asm/rtas.h
 
 struct cc_workarea {
-   u32 drc_index;
-   u32 zero;
-   u32 name_offset;
-   u32 prop_length;
-   u32 prop_offset;
+   __be32  drc_index;
+   __be32  zero;
+   __be32  name_offset;
+   __be32  prop_length;
+   __be32  prop_offset;
 };
 
 void dlpar_free_cc_property(struct property *prop)
@@ -48,11 +48,11 @@ static struct property *dlpar_parse_cc_property(struct 
cc_workarea *ccwa)
if (!prop)
return NULL;
 
-   name = (char *)ccwa + ccwa-name_offset;
+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
prop-name = kstrdup(name, GFP_KERNEL);
 
-   prop-length = ccwa-prop_length;
-   value = (char *)ccwa + ccwa-prop_offset;
+   prop-length = be32_to_cpu(ccwa-prop_length);
+   value = (char *)ccwa + be32_to_cpu(ccwa-prop_offset);
prop-value = kmemdup(value, prop-length, GFP_KERNEL);
if (!prop-value) {
dlpar_free_cc_property(prop);
@@ -78,7 +78,7 @@ static struct device_node *dlpar_parse_cc_node(struct 
cc_workarea *ccwa,
if (!dn)
return NULL;
 
-   name = (char *)ccwa + ccwa-name_offset;
+   name = (char *)ccwa + be32_to_cpu(ccwa-name_offset);
dn-full_name = kasprintf(GFP_KERNEL, %s/%s, path, name);
if (!dn-full_name) {
kfree(dn);
@@ -125,7 +125,7 @@ void dlpar_free_cc_nodes(struct device_node *dn)
 #define CALL_AGAIN -2
 #define ERR_CFG_USE -9003
 
-struct device_node *dlpar_configure_connector(u32 drc_index,
+struct device_node *dlpar_configure_connector(__be32 drc_index,
  struct device_node *parent)
 {
struct device_node *dn;
@@ -411,7 +411,7 @@ static ssize_t dlpar_cpu_probe(const char *buf, size_t 
count)
if (!parent)
return -ENODEV;
 
-   dn = dlpar_configure_connector(drc_index, parent);
+   dn = dlpar_configure_connector(cpu_to_be32(drc_index), parent);
if (!dn)
return -EINVAL;
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 20d6297..447f8c6 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -247,7 +247,7 @@ static int pseries_add_processor(struct device_node *np)
unsigned int cpu;
cpumask_var_t candidate_mask, tmp;
int err = -ENOSPC, len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
 
intserv = of_get_property(np, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -293,7 +293,7 @@ static int pseries_add_processor(struct device_node *np)
for_each_cpu(cpu, tmp) {
BUG_ON(cpu_present(cpu));
set_cpu_present(cpu, true);
-   set_hard_smp_processor_id(cpu, *intserv++);
+   set_hard_smp_processor_id(cpu, be32_to_cpu(*intserv++));
}
err = 0;
 out_unlock:
diff --git a/arch/powerpc/platforms/pseries/pseries.h 
b/arch/powerpc/platforms/pseries/pseries.h
index 361add6..1796c54 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -56,7 +56,8 @@ extern void hvc_vio_init_early(void);
 /* Dynamic logical Partitioning/Mobility */
 extern void dlpar_free_cc_nodes(struct device_node *);
 extern void dlpar_free_cc_property(struct property *);
-extern struct device_node *dlpar_configure_connector(u32, struct device_node 
*);
+extern struct device_node *dlpar_configure_connector(__be32,
+   struct device_node *);
 extern int dlpar_attach_node(struct device_node *);
 extern int dlpar_detach_node(struct device_node *);
 
-- 
1.8.5.2

[PATCH v3 3/3] pseries: Fix endian issues in cpu hot-removal

[Thomas Falcon]

When removing a cpu, this patch makes sure that values
gotten from or passed to firmware are in the correct
endian format.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
Changes in v3:

 drc_index in dlpar_cpu_release is no longer const to
 fix compilation error found by Bharata Rao
---
 arch/powerpc/platforms/pseries/dlpar.c   | 20 +++-
 arch/powerpc/platforms/pseries/hotplug-cpu.c | 10 ++
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
b/arch/powerpc/platforms/pseries/dlpar.c
index 187e4eb..0fad5b6 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -444,7 +444,8 @@ static int dlpar_offline_cpu(struct device_node *dn)
int rc = 0;
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(dn, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -454,8 +455,9 @@ static int dlpar_offline_cpu(struct device_node *dn)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
 
if (get_cpu_current_state(cpu) == CPU_STATE_OFFLINE)
@@ -477,14 +479,14 @@ static int dlpar_offline_cpu(struct device_node *dn)
 * Upgrade it's state to CPU_STATE_OFFLINE.
 */
set_preferred_offline_state(cpu, CPU_STATE_OFFLINE);
-   BUG_ON(plpar_hcall_norets(H_PROD, intserv[i])
+   BUG_ON(plpar_hcall_norets(H_PROD, thread)
!= H_SUCCESS);
__cpu_die(cpu);
break;
}
if (cpu == num_possible_cpus())
printk(KERN_WARNING Could not find cpu to offline 
-  with physical id 0x%x\n, intserv[i]);
+  with physical id 0x%x\n, thread);
}
cpu_maps_update_done();
 
@@ -496,15 +498,15 @@ out:
 static ssize_t dlpar_cpu_release(const char *buf, size_t count)
 {
struct device_node *dn;
-   const u32 *drc_index;
+   u32 drc_index;
int rc;
 
dn = of_find_node_by_path(buf);
if (!dn)
return -EINVAL;
 
-   drc_index = of_get_property(dn, ibm,my-drc-index, NULL);
-   if (!drc_index) {
+   rc = of_property_read_u32(dn, ibm,my-drc-index, drc_index);
+   if (rc) {
of_node_put(dn);
return -EINVAL;
}
@@ -515,7 +517,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
return -EINVAL;
}
 
-   rc = dlpar_release_drc(*drc_index);
+   rc = dlpar_release_drc(drc_index);
if (rc) {
of_node_put(dn);
return rc;
@@ -523,7 +525,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t 
count)
 
rc = dlpar_detach_node(dn);
if (rc) {
-   dlpar_acquire_drc(*drc_index);
+   dlpar_acquire_drc(drc_index);
return rc;
}
 
diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index 447f8c6..5c375f9 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -90,7 +90,7 @@ static void rtas_stop_self(void)
 {
static struct rtas_args args = {
.nargs = 0,
-   .nret = 1,
+   .nret = cpu_to_be32(1),
.rets = args.args[0],
};
 
@@ -312,7 +312,8 @@ static void pseries_remove_processor(struct device_node *np)
 {
unsigned int cpu;
int len, nthreads, i;
-   const u32 *intserv;
+   const __be32 *intserv;
+   u32 thread;
 
intserv = of_get_property(np, ibm,ppc-interrupt-server#s, len);
if (!intserv)
@@ -322,8 +323,9 @@ static void pseries_remove_processor(struct device_node *np)
 
cpu_maps_update_begin();
for (i = 0; i  nthreads; i++) {
+   thread = be32_to_cpu(intserv[i]);
for_each_present_cpu(cpu) {
-   if (get_hard_smp_processor_id(cpu) != intserv[i])
+   if (get_hard_smp_processor_id(cpu) != thread)
continue;
BUG_ON(cpu_online(cpu));
set_cpu_present(cpu, false);
@@ -332,7 +334,7 @@ static void pseries_remove_processor(struct device_node *np)
}
if (cpu = nr_cpu_ids)
printk

[PATCHi v2] ibmveth: Add function to enable live MAC address changes

[Thomas Falcon]

Add a function that will enable changing the MAC address
of an ibmveth interface while it is still running.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
v2:
   If h_change_logical_lan_mac fails, dev-dev_addr will not be changed.

 drivers/net/ethernet/ibm/ibmveth.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 21978cc..b6ac676 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1327,6 +1327,29 @@ static unsigned long ibmveth_get_desired_dma(struct 
vio_dev *vdev)
return ret;
 }
 
+static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   struct sockaddr *addr = p;
+   u64 mac_address;
+   int rc;
+
+   if (!is_valid_ether_addr(addr-sa_data))
+   return -EADDRNOTAVAIL;
+
+   mac_address = ibmveth_encode_mac_addr(addr-sa_data);
+   rc = h_change_logical_lan_mac(adapter-vdev-unit_address, mac_address);
+   if (rc) {
+   netdev_err(adapter-netdev, h_change_logical_lan_mac failed 
+  with rc=%d\n, rc);
+   return rc;
+   }
+
+   ether_addr_copy(dev-dev_addr, addr-sa_data);
+
+   return 0;
+}
+
 static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_open   = ibmveth_open,
.ndo_stop   = ibmveth_close,
@@ -1337,7 +1360,7 @@ static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_fix_features   = ibmveth_fix_features,
.ndo_set_features   = ibmveth_set_features,
.ndo_validate_addr  = eth_validate_addr,
-   .ndo_set_mac_address= eth_mac_addr,
+   .ndo_set_mac_address= ibmveth_set_mac_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ibmveth_poll_controller,
 #endif
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCHi v2] ibmveth: Add function to enable live MAC address changes

[Thomas Falcon]

On 02/28/2015 02:59 AM, Jiri Pirko wrote:
 Sat, Feb 28, 2015 at 06:56:04AM CET, tlfal...@linux.vnet.ibm.com wrote:
 Add a function that will enable changing the MAC address
 of an ibmveth interface while it is still running.

 Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
 ---
 v2:
   If h_change_logical_lan_mac fails, dev-dev_addr will not be changed.

 drivers/net/ethernet/ibm/ibmveth.c | 25 -
 1 file changed, 24 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
 b/drivers/net/ethernet/ibm/ibmveth.c
 index 21978cc..b6ac676 100644
 --- a/drivers/net/ethernet/ibm/ibmveth.c
 +++ b/drivers/net/ethernet/ibm/ibmveth.c
 @@ -1327,6 +1327,29 @@ static unsigned long ibmveth_get_desired_dma(struct 
 vio_dev *vdev)
  return ret;
 }

 +static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
 +{
 +struct ibmveth_adapter *adapter = netdev_priv(dev);
 +struct sockaddr *addr = p;
 +u64 mac_address;
 +int rc;
 +
 +if (!is_valid_ether_addr(addr-sa_data))
 +return -EADDRNOTAVAIL;
 +
 +mac_address = ibmveth_encode_mac_addr(addr-sa_data);
 +rc = h_change_logical_lan_mac(adapter-vdev-unit_address, mac_address);
 +if (rc) {
 +netdev_err(adapter-netdev, h_change_logical_lan_mac failed 
 +   with rc=%d\n, rc);
 Please do not wrap text in message. For that, 80-char limit does not apply.

I will send a new patch fixing this shortly.  Thanks to you, Brian, and Dave 
for reviewing this patch.



 +return rc;
 +}
 +
 +ether_addr_copy(dev-dev_addr, addr-sa_data);
 +
 +return 0;
 +}
 +
 static const struct net_device_ops ibmveth_netdev_ops = {
  .ndo_open   = ibmveth_open,
  .ndo_stop   = ibmveth_close,
 @@ -1337,7 +1360,7 @@ static const struct net_device_ops ibmveth_netdev_ops 
 = {
  .ndo_fix_features   = ibmveth_fix_features,
  .ndo_set_features   = ibmveth_set_features,
  .ndo_validate_addr  = eth_validate_addr,
 -.ndo_set_mac_address= eth_mac_addr,
 +.ndo_set_mac_address= ibmveth_set_mac_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER
  .ndo_poll_controller= ibmveth_poll_controller,
 #endif
 -- 
 1.8.3.1


___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH v3] ibmveth: Add function to enable live MAC address changes

[Thomas Falcon]

Add a function that will enable changing the MAC address
of an ibmveth interface while it is still running.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
v3:
   removed text wrapping in error message
v2:
   If h_change_logical_lan_mac fails, dev-dev_addr will not be changed.

 drivers/net/ethernet/ibm/ibmveth.c | 24 +++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 21978cc..072426a 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1327,6 +1327,28 @@ static unsigned long ibmveth_get_desired_dma(struct 
vio_dev *vdev)
return ret;
 }
 
+static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   struct sockaddr *addr = p;
+   u64 mac_address;
+   int rc;
+
+   if (!is_valid_ether_addr(addr-sa_data))
+   return -EADDRNOTAVAIL;
+
+   mac_address = ibmveth_encode_mac_addr(addr-sa_data);
+   rc = h_change_logical_lan_mac(adapter-vdev-unit_address, mac_address);
+   if (rc) {
+   netdev_err(adapter-netdev, h_change_logical_lan_mac failed 
with rc=%d\n, rc);
+   return rc;
+   }
+
+   ether_addr_copy(dev-dev_addr, addr-sa_data);
+
+   return 0;
+}
+
 static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_open   = ibmveth_open,
.ndo_stop   = ibmveth_close,
@@ -1337,7 +1359,7 @@ static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_fix_features   = ibmveth_fix_features,
.ndo_set_features   = ibmveth_set_features,
.ndo_validate_addr  = eth_validate_addr,
-   .ndo_set_mac_address= eth_mac_addr,
+   .ndo_set_mac_address= ibmveth_set_mac_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ibmveth_poll_controller,
 #endif
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH] ibmveth: Add function to enable live MAC address changes

[Thomas Falcon]

Add a function that will enable changing the MAC address
of an ibmveth interface while it is still running.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 20 +++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 21978cc..6e44357 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1327,6 +1327,24 @@ static unsigned long ibmveth_get_desired_dma(struct 
vio_dev *vdev)
return ret;
 }
 
+static int ibmveth_set_mac_addr(struct net_device *dev, void *p)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   struct sockaddr *addr = p;
+   u64 mac_address;
+   int rc;
+
+   if (!is_valid_ether_addr(addr-sa_data))
+   return -EADDRNOTAVAIL;
+
+   ether_addr_copy(dev-dev_addr, addr-sa_data);
+
+   mac_address = ibmveth_encode_mac_addr(dev-dev_addr);
+   rc = h_change_logical_lan_mac(adapter-vdev-unit_address, mac_address);
+
+   return rc;
+}
+
 static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_open   = ibmveth_open,
.ndo_stop   = ibmveth_close,
@@ -1337,7 +1355,7 @@ static const struct net_device_ops ibmveth_netdev_ops = {
.ndo_fix_features   = ibmveth_fix_features,
.ndo_set_features   = ibmveth_set_features,
.ndo_validate_addr  = eth_validate_addr,
-   .ndo_set_mac_address= eth_mac_addr,
+   .ndo_set_mac_address= ibmveth_set_mac_addr,
 #ifdef CONFIG_NET_POLL_CONTROLLER
.ndo_poll_controller= ibmveth_poll_controller,
 #endif
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH 4/5] ibmveth: Add support for Large Receive Offload

[Thomas Falcon]

On 04/14/2015 05:00 PM, Eric Dumazet wrote:
 On Tue, 2015-04-14 at 15:35 -0500, Thomas Falcon wrote:
 Enables receiving large packets from other LPARs. These packets
 have a -1 IP header checksum, so we must recalculate to have
 a valid checksum.

 Signed-off-by: Brian King brk...@linux.vnet.ibm.com
 Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
 ---
  drivers/net/ethernet/ibm/ibmveth.c | 15 ++-
  1 file changed, 14 insertions(+), 1 deletion(-)

 diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
 b/drivers/net/ethernet/ibm/ibmveth.c
 index 08970c7..05eaca6a 100644
 --- a/drivers/net/ethernet/ibm/ibmveth.c
 +++ b/drivers/net/ethernet/ibm/ibmveth.c
 @@ -1092,6 +1092,7 @@ static int ibmveth_poll(struct napi_struct *napi, int 
 budget)
  struct net_device *netdev = adapter-netdev;
  int frames_processed = 0;
  unsigned long lpar_rc;
 +struct iphdr *iph;
  
  restart_poll:
  while (frames_processed  budget) {
 @@ -1134,8 +1135,20 @@ restart_poll:
  skb_put(skb, length);
  skb-protocol = eth_type_trans(skb, netdev);
  
 -if (csum_good)
 +if (csum_good) {
  skb-ip_summed = CHECKSUM_UNNECESSARY;
 +if (be16_to_cpu(skb-protocol) == ETH_P_IP) {
 +skb_set_network_header(skb, 0);
 +skb_set_transport_header(skb, 
 sizeof(struct iphdr));
 +iph = ip_hdr(skb);
 +
 +/* If the IP checksum is not offloaded 
 and if the packet
 + *  is large send, the checksum must be 
 rebuilt.
 + */
 +if (iph-check == 0x)
 +iph-check = 
 ip_fast_csum((unsigned char *)iph, iph-ihl);

 How can this possibly work ?

 Normally you would have to set iph-check to 0 before calling
 ip_fast_csum(), as done in ip_send_check()
I don't have an answer for why I weren't seeing any problems while testing 
this, but I'll go back and set iph-check to zero and retest.  Thanks for 
noticing this.


 ___
 Linuxppc-dev mailing list
 Linuxppc-dev@lists.ozlabs.org
 https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCHv2] ibmveth: Fix off-by-one error in ibmveth_change_mtu()

[Thomas Falcon]

On 04/20/2015 08:07 PM, David Gibson wrote:
 AFAIK the PAPR document which defines the virtual device interface used by
 the ibmveth driver doesn't specify a specific maximum MTU.  So, in the
 ibmveth driver, the maximum allowed MTU is determined by the maximum
 allocated buffer size of 64k (corresponding to one page in the common case)
 minus the per-buffer overhead IBMVETH_BUFF_OH (which has value 22 for 14
 bytes of ethernet header, plus 8 bytes for an opaque handle).

 This suggests a maximum allowable MTU of 65514 bytes, but in fact the
 driver only permits a maximum MTU of 65513.  This is because there is a 
 instead of an = in ibmveth_change_mtu(), which only permits an MTU which
 is strictly smaller than the buffer size, rather than allowing the buffer
 to be completely filled.

 This patch fixes the buglet.

Thanks!

Acked-by: Thomas Falcon tlfal...@linux.vnet.ibm.com


 Signed-off-by: David Gibson da...@gibson.dropbear.id.au1
 ---
  drivers/net/ethernet/ibm/ibmveth.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

 Changes since v1:
  * Fixed a second instance of the same off-by-one error.  Thanks to
Thomas Falcon for spotting this.

 diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
 b/drivers/net/ethernet/ibm/ibmveth.c
 index cd7675a..1813476 100644
 --- a/drivers/net/ethernet/ibm/ibmveth.c
 +++ b/drivers/net/ethernet/ibm/ibmveth.c
 @@ -1238,7 +1238,7 @@ static int ibmveth_change_mtu(struct net_device *dev, 
 int new_mtu)
   return -EINVAL;
  
   for (i = 0; i  IBMVETH_NUM_BUFF_POOLS; i++)
 - if (new_mtu_oh  adapter-rx_buff_pool[i].buff_size)
 + if (new_mtu_oh = adapter-rx_buff_pool[i].buff_size)
   break;
  
   if (i == IBMVETH_NUM_BUFF_POOLS)
 @@ -1257,7 +1257,7 @@ static int ibmveth_change_mtu(struct net_device *dev, 
 int new_mtu)
   for (i = 0; i  IBMVETH_NUM_BUFF_POOLS; i++) {
   adapter-rx_buff_pool[i].active = 1;
  
 - if (new_mtu_oh  adapter-rx_buff_pool[i].buff_size) {
 + if (new_mtu_oh = adapter-rx_buff_pool[i].buff_size) {
   dev-mtu = new_mtu;
   vio_cmo_set_dev_desired(viodev,
   ibmveth_get_desired_dma

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v3 4/4] ibmveth: Add support for Large Receive Offload

[Thomas Falcon]

Enables receiving large packets from other LPARs. These packets
have a -1 IP header checksum, so we must recalculate to have
a valid checksum.

Signed-off-by: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
v3:
 -Removed code setting network and transport headers
 -get IP header from skb data
  Thanks again to Eric Dumazet

v2:
 -Included statistics that were previously in a separate patch
 -Zeroed the IP header checksum before calling ip_fast_csum
  Thanks to Eric Dumazet.

 drivers/net/ethernet/ibm/ibmveth.c | 17 -
 drivers/net/ethernet/ibm/ibmveth.h |  1 +
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index f0ec4e5..4468124 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -101,6 +101,7 @@ struct ibmveth_stat ibmveth_stats[] = {
{ fw_enabled_ipv4_csum, IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
{ fw_enabled_ipv6_csum, IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
{ tx_large_packets, IBMVETH_STAT_OFF(tx_large_packets) },
+   { rx_large_packets, IBMVETH_STAT_OFF(rx_large_packets) }
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -1094,6 +1095,7 @@ static int ibmveth_poll(struct napi_struct *napi, int 
budget)
struct net_device *netdev = adapter-netdev;
int frames_processed = 0;
unsigned long lpar_rc;
+   struct iphdr *iph;
 
 restart_poll:
while (frames_processed  budget) {
@@ -1136,8 +1138,21 @@ restart_poll:
skb_put(skb, length);
skb-protocol = eth_type_trans(skb, netdev);
 
-   if (csum_good)
+   if (csum_good) {
skb-ip_summed = CHECKSUM_UNNECESSARY;
+   if (be16_to_cpu(skb-protocol) == ETH_P_IP) {
+   iph = (struct iphdr *)skb-data;
+
+   /* If the IP checksum is not offloaded 
and if the packet
+*  is large send, the checksum must be 
rebuilt.
+*/
+   if (iph-check == 0x) {
+   iph-check = 0;
+   iph-check = 
ip_fast_csum((unsigned char *)iph, iph-ihl);
+   adapter-rx_large_packets++;
+   }
+   }
+   }
 
napi_gro_receive(napi, skb);/* send it up */
 
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 56d1e22..41dedb1 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -162,6 +162,7 @@ struct ibmveth_adapter {
 u64 tx_map_failed;
 u64 tx_send_failed;
 u64 tx_large_packets;
+u64 rx_large_packets;
 };
 
 /*
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v3 1/4] ibmveth: change rx buffer default allocation for CMO

[Thomas Falcon]

This patch enables 64k rx buffer pools by default.  If Cooperative
Memory Overcommitment (CMO) is enabled, the number of 64k buffers
is reduced to save memory.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 3 +++
 drivers/net/ethernet/ibm/ibmveth.h | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index cd7675a..0210622 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1424,6 +1424,9 @@ static int ibmveth_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
 
memcpy(netdev-dev_addr, mac_addr_p, ETH_ALEN);
 
+   if (firmware_has_feature(FW_FEATURE_CMO))
+   memcpy(pool_count, pool_count_cmo, sizeof(pool_count));
+
for (i = 0; i  IBMVETH_NUM_BUFF_POOLS; i++) {
struct kobject *kobj = adapter-rx_buff_pool[i].kobj;
int error;
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 1f37499..0dc664b 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -104,7 +104,8 @@ static inline long h_illan_attributes(unsigned long 
unit_address,
 
 static int pool_size[] = { 512, 1024 * 2, 1024 * 16, 1024 * 32, 1024 * 64 };
 static int pool_count[] = { 256, 512, 256, 256, 256 };
-static int pool_active[] = { 1, 1, 0, 0, 0};
+static int pool_count_cmo[] = { 256, 512, 256, 256, 64 };
+static int pool_active[] = { 1, 1, 0, 0, 1};
 
 #define IBM_VETH_INVALID_MAP ((u16)0x)
 
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v3 3/4] ibmveth: Add GRO support

[Thomas Falcon]

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 25cfc26..f0ec4e5 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1139,7 +1139,7 @@ restart_poll:
if (csum_good)
skb-ip_summed = CHECKSUM_UNNECESSARY;
 
-   netif_receive_skb(skb); /* send it up */
+   napi_gro_receive(napi, skb);/* send it up */
 
netdev-stats.rx_packets++;
netdev-stats.rx_bytes += length;
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 1/4] ibmveth: change rx buffer default allocation for CMO

[Thomas Falcon]

This patch enables 64k rx buffer pools by default.  If Cooperative
Memory Overcommitment (CMO) is enabled, the number of 64k buffers
is reduced to save memory.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 3 +++
 drivers/net/ethernet/ibm/ibmveth.h | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index cd7675a..0210622 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1424,6 +1424,9 @@ static int ibmveth_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
 
memcpy(netdev-dev_addr, mac_addr_p, ETH_ALEN);
 
+   if (firmware_has_feature(FW_FEATURE_CMO))
+   memcpy(pool_count, pool_count_cmo, sizeof(pool_count));
+
for (i = 0; i  IBMVETH_NUM_BUFF_POOLS; i++) {
struct kobject *kobj = adapter-rx_buff_pool[i].kobj;
int error;
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 1f37499..0dc664b 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -104,7 +104,8 @@ static inline long h_illan_attributes(unsigned long 
unit_address,
 
 static int pool_size[] = { 512, 1024 * 2, 1024 * 16, 1024 * 32, 1024 * 64 };
 static int pool_count[] = { 256, 512, 256, 256, 256 };
-static int pool_active[] = { 1, 1, 0, 0, 0};
+static int pool_count_cmo[] = { 256, 512, 256, 256, 64 };
+static int pool_active[] = { 1, 1, 0, 0, 1};
 
 #define IBM_VETH_INVALID_MAP ((u16)0x)
 
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 2/4] ibmveth: Add support for TSO

[Thomas Falcon]

Add support for TSO.  TSO is turned off by default and
must be enabled and configured by the user.  The driver
version number is increased so that users can be sure
that they are using ibmveth with TSO support.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
v2: Included statistics that were previously in a separate patch

 drivers/net/ethernet/ibm/ibmveth.c | 19 ++-
 drivers/net/ethernet/ibm/ibmveth.h |  1 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 0210622..25cfc26 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -58,7 +58,7 @@ static struct kobj_type ktype_veth_pool;
 
 static const char ibmveth_driver_name[] = ibmveth;
 static const char ibmveth_driver_string[] = IBM Power Virtual Ethernet 
Driver;
-#define ibmveth_driver_version 1.04
+#define ibmveth_driver_version 1.05
 
 MODULE_AUTHOR(Santiago Leon san...@linux.vnet.ibm.com);
 MODULE_DESCRIPTION(IBM Power Virtual Ethernet Driver);
@@ -100,6 +100,7 @@ struct ibmveth_stat ibmveth_stats[] = {
{ tx_send_failed, IBMVETH_STAT_OFF(tx_send_failed) },
{ fw_enabled_ipv4_csum, IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
{ fw_enabled_ipv6_csum, IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
+   { tx_large_packets, IBMVETH_STAT_OFF(tx_large_packets) },
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -852,6 +853,10 @@ static int ibmveth_set_features(struct net_device *dev,
struct ibmveth_adapter *adapter = netdev_priv(dev);
int rx_csum = !!(features  NETIF_F_RXCSUM);
int rc;
+   netdev_features_t changed = features ^ dev-features;
+
+   if (features  NETIF_F_TSO  changed)
+   netdev_info(dev, TSO feature requires all partitions to have 
updated driver);
 
if (rx_csum == adapter-rx_csum)
return 0;
@@ -1035,6 +1040,15 @@ retry_bounce:
descs[i+1].fields.address = dma_addr;
}
 
+   if (skb_is_gso(skb)  !skb_is_gso_v6(skb)) {
+   /* Put -1 in the IP checksum to tell phyp it
+*  is a largesend packet and put the mss in the TCP checksum.
+*/
+   ip_hdr(skb)-check = 0x;
+   tcp_hdr(skb)-check = cpu_to_be16(skb_shinfo(skb)-gso_size);
+   adapter-tx_large_packets++;
+   }
+
if (ibmveth_send(adapter, descs)) {
adapter-tx_send_failed++;
netdev-stats.tx_dropped++;
@@ -1422,6 +1436,9 @@ static int ibmveth_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
netdev-features |= netdev-hw_features;
 
+   /* TSO is disabled by default */
+   netdev-hw_features |= NETIF_F_TSO;
+
memcpy(netdev-dev_addr, mac_addr_p, ETH_ALEN);
 
if (firmware_has_feature(FW_FEATURE_CMO))
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 0dc664b..56d1e22 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -161,6 +161,7 @@ struct ibmveth_adapter {
 u64 rx_no_buffer;
 u64 tx_map_failed;
 u64 tx_send_failed;
+u64 tx_large_packets;
 };
 
 /*
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 4/4] ibmveth: Add support for Large Receive Offload

[Thomas Falcon]

Enables receiving large packets from other LPARs. These packets
have a -1 IP header checksum, so we must recalculate to have
a valid checksum.

Signed-off-by: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
v2: 
 -Included statistics that were previously in a separate patch
 -Zeroed the IP header checksum before calling ip_fast_csum
  Thanks to Eric Dumazet.

 drivers/net/ethernet/ibm/ibmveth.c | 19 ++-
 drivers/net/ethernet/ibm/ibmveth.h |  1 +
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index f0ec4e5..3bab896 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -101,6 +101,7 @@ struct ibmveth_stat ibmveth_stats[] = {
{ fw_enabled_ipv4_csum, IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
{ fw_enabled_ipv6_csum, IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
{ tx_large_packets, IBMVETH_STAT_OFF(tx_large_packets) },
+   { rx_large_packets, IBMVETH_STAT_OFF(rx_large_packets) }
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -1094,6 +1095,7 @@ static int ibmveth_poll(struct napi_struct *napi, int 
budget)
struct net_device *netdev = adapter-netdev;
int frames_processed = 0;
unsigned long lpar_rc;
+   struct iphdr *iph;
 
 restart_poll:
while (frames_processed  budget) {
@@ -1136,8 +1138,23 @@ restart_poll:
skb_put(skb, length);
skb-protocol = eth_type_trans(skb, netdev);
 
-   if (csum_good)
+   if (csum_good) {
skb-ip_summed = CHECKSUM_UNNECESSARY;
+   if (be16_to_cpu(skb-protocol) == ETH_P_IP) {
+   skb_set_network_header(skb, 0);
+   skb_set_transport_header(skb, 
sizeof(struct iphdr));
+   iph = ip_hdr(skb);
+
+   /* If the IP checksum is not offloaded 
and if the packet
+*  is large send, the checksum must be 
rebuilt.
+*/
+   if (iph-check == 0x) {
+   iph-check = 0;
+   iph-check = 
ip_fast_csum((unsigned char *)iph, iph-ihl);
+   adapter-rx_large_packets++;
+   }
+   }
+   }
 
napi_gro_receive(napi, skb);/* send it up */
 
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 56d1e22..41dedb1 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -162,6 +162,7 @@ struct ibmveth_adapter {
 u64 tx_map_failed;
 u64 tx_send_failed;
 u64 tx_large_packets;
+u64 rx_large_packets;
 };
 
 /*
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next v2 3/4] ibmveth: Add GRO support

[Thomas Falcon]

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 25cfc26..f0ec4e5 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1139,7 +1139,7 @@ restart_poll:
if (csum_good)
skb-ip_summed = CHECKSUM_UNNECESSARY;
 
-   netif_receive_skb(skb); /* send it up */
+   napi_gro_receive(napi, skb);/* send it up */
 
netdev-stats.rx_packets++;
netdev-stats.rx_bytes += length;
-- 
1.9.3

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 4/5] ibmveth: Add support for Large Receive Offload

[Thomas Falcon]

Enables receiving large packets from other LPARs. These packets
have a -1 IP header checksum, so we must recalculate to have
a valid checksum.

Signed-off-by: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 08970c7..05eaca6a 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1092,6 +1092,7 @@ static int ibmveth_poll(struct napi_struct *napi, int 
budget)
struct net_device *netdev = adapter-netdev;
int frames_processed = 0;
unsigned long lpar_rc;
+   struct iphdr *iph;
 
 restart_poll:
while (frames_processed  budget) {
@@ -1134,8 +1135,20 @@ restart_poll:
skb_put(skb, length);
skb-protocol = eth_type_trans(skb, netdev);
 
-   if (csum_good)
+   if (csum_good) {
skb-ip_summed = CHECKSUM_UNNECESSARY;
+   if (be16_to_cpu(skb-protocol) == ETH_P_IP) {
+   skb_set_network_header(skb, 0);
+   skb_set_transport_header(skb, 
sizeof(struct iphdr));
+   iph = ip_hdr(skb);
+
+   /* If the IP checksum is not offloaded 
and if the packet
+*  is large send, the checksum must be 
rebuilt.
+*/
+   if (iph-check == 0x)
+   iph-check = 
ip_fast_csum((unsigned char *)iph, iph-ihl);
+   }
+   }
 
napi_gro_receive(napi, skb);/* send it up */
 
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 2/5] ibmveth: Add support for TSO

[Thomas Falcon]

Add support for TSO.  TSO is turned off by default and
must be enabled and configured by the user.  The driver
version number is increased so that users can be sure
that they are using ibmveth with TSO support.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 0210622..2911a57 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -58,7 +58,7 @@ static struct kobj_type ktype_veth_pool;
 
 static const char ibmveth_driver_name[] = ibmveth;
 static const char ibmveth_driver_string[] = IBM Power Virtual Ethernet 
Driver;
-#define ibmveth_driver_version 1.04
+#define ibmveth_driver_version 1.05
 
 MODULE_AUTHOR(Santiago Leon san...@linux.vnet.ibm.com);
 MODULE_DESCRIPTION(IBM Power Virtual Ethernet Driver);
@@ -852,6 +852,10 @@ static int ibmveth_set_features(struct net_device *dev,
struct ibmveth_adapter *adapter = netdev_priv(dev);
int rx_csum = !!(features  NETIF_F_RXCSUM);
int rc;
+   netdev_features_t changed = features ^ dev-features;
+
+   if (features  NETIF_F_TSO  changed)
+   netdev_info(dev, TSO feature requires all partitions to have 
updated driver);
 
if (rx_csum == adapter-rx_csum)
return 0;
@@ -1035,6 +1039,14 @@ retry_bounce:
descs[i+1].fields.address = dma_addr;
}
 
+   if (skb_is_gso(skb)  !skb_is_gso_v6(skb)) {
+   /* Put -1 in the IP checksum to tell phyp it
+*  is a largesend packet and put the mss in the TCP checksum.
+*/
+   ip_hdr(skb)-check = 0x;
+   tcp_hdr(skb)-check = cpu_to_be16(skb_shinfo(skb)-gso_size);
+   }
+
if (ibmveth_send(adapter, descs)) {
adapter-tx_send_failed++;
netdev-stats.tx_dropped++;
@@ -1422,6 +1434,9 @@ static int ibmveth_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
netdev-features |= netdev-hw_features;
 
+   /* TSO is disabled by default */
+   netdev-hw_features |= NETIF_F_TSO;
+
memcpy(netdev-dev_addr, mac_addr_p, ETH_ALEN);
 
if (firmware_has_feature(FW_FEATURE_CMO))
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 3/5] ibmveth: Add GRO support

[Thomas Falcon]

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 2911a57..08970c7 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1137,7 +1137,7 @@ restart_poll:
if (csum_good)
skb-ip_summed = CHECKSUM_UNNECESSARY;
 
-   netif_receive_skb(skb); /* send it up */
+   napi_gro_receive(napi, skb);/* send it up */
 
netdev-stats.rx_packets++;
netdev-stats.rx_bytes += length;
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 1/5] ibmveth: change rx buffer default allocation for CMO

[Thomas Falcon]

This patch enables 64k rx buffer pools by default.  If Cooperative
Memory Overcommitment (CMO) is enabled, the number of 64k buffers
is reduced to save memory.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 3 +++
 drivers/net/ethernet/ibm/ibmveth.h | 3 ++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index cd7675a..0210622 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -1424,6 +1424,9 @@ static int ibmveth_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
 
memcpy(netdev-dev_addr, mac_addr_p, ETH_ALEN);
 
+   if (firmware_has_feature(FW_FEATURE_CMO))
+   memcpy(pool_count, pool_count_cmo, sizeof(pool_count));
+
for (i = 0; i  IBMVETH_NUM_BUFF_POOLS; i++) {
struct kobject *kobj = adapter-rx_buff_pool[i].kobj;
int error;
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 1f37499..0dc664b 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -104,7 +104,8 @@ static inline long h_illan_attributes(unsigned long 
unit_address,
 
 static int pool_size[] = { 512, 1024 * 2, 1024 * 16, 1024 * 32, 1024 * 64 };
 static int pool_count[] = { 256, 512, 256, 256, 256 };
-static int pool_active[] = { 1, 1, 0, 0, 0};
+static int pool_count_cmo[] = { 256, 512, 256, 256, 64 };
+static int pool_active[] = { 1, 1, 0, 0, 1};
 
 #define IBM_VETH_INVALID_MAP ((u16)0x)
 
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH 5/5] ibmveth: Add ethtool statistics for tx and rx large packets

[Thomas Falcon]

This patch includes counters for transmitted and received large
packets.

Cc: Brian King brk...@linux.vnet.ibm.com
Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c | 4 
 drivers/net/ethernet/ibm/ibmveth.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 05eaca6a..39ab41e 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -100,6 +100,8 @@ struct ibmveth_stat ibmveth_stats[] = {
{ tx_send_failed, IBMVETH_STAT_OFF(tx_send_failed) },
{ fw_enabled_ipv4_csum, IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
{ fw_enabled_ipv6_csum, IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
+   { tx_large_packets, IBMVETH_STAT_OFF(tx_large_packets) },
+   { rx_large_packets, IBMVETH_STAT_OFF(rx_large_packets) }
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -1045,6 +1047,7 @@ retry_bounce:
 */
ip_hdr(skb)-check = 0x;
tcp_hdr(skb)-check = cpu_to_be16(skb_shinfo(skb)-gso_size);
+   adapter-tx_large_packets++;
}
 
if (ibmveth_send(adapter, descs)) {
@@ -1147,6 +1150,7 @@ restart_poll:
 */
if (iph-check == 0x)
iph-check = 
ip_fast_csum((unsigned char *)iph, iph-ihl);
+   adapter-rx_large_packets++;
}
}
 
diff --git a/drivers/net/ethernet/ibm/ibmveth.h 
b/drivers/net/ethernet/ibm/ibmveth.h
index 0dc664b..41dedb1 100644
--- a/drivers/net/ethernet/ibm/ibmveth.h
+++ b/drivers/net/ethernet/ibm/ibmveth.h
@@ -161,6 +161,8 @@ struct ibmveth_adapter {
 u64 rx_no_buffer;
 u64 tx_map_failed;
 u64 tx_send_failed;
+u64 tx_large_packets;
+u64 rx_large_packets;
 };
 
 /*
-- 
1.8.3.1

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[PATCH net-next] ibmveth: add support for TSO6

[Thomas Falcon]

This patch adds support for a new method of signalling the firmware
that TSO packets are being sent. The new method removes the need to
alter the ip and tcp checksums and allows TSO6 support.

Signed-off-by: Thomas Falcon tlfal...@linux.vnet.ibm.com
---
 drivers/net/ethernet/ibm/ibmveth.c |  145 +--
 drivers/net/ethernet/ibm/ibmveth.h |   18 -
 2 files changed, 135 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index 29bbb62..d4b92cd 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -79,6 +79,11 @@ static unsigned int rx_flush __read_mostly = 0;
 module_param(rx_flush, uint, 0644);
 MODULE_PARM_DESC(rx_flush, Flush receive buffers before use);
 
+static bool old_large_send __read_mostly;
+module_param(old_large_send, bool, S_IRUGO);
+MODULE_PARM_DESC(old_large_send,
+   Use old large send method on firmware that supports the new method);
+
 struct ibmveth_stat {
char name[ETH_GSTRING_LEN];
int offset;
@@ -101,7 +106,8 @@ struct ibmveth_stat ibmveth_stats[] = {
{ fw_enabled_ipv4_csum, IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
{ fw_enabled_ipv6_csum, IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
{ tx_large_packets, IBMVETH_STAT_OFF(tx_large_packets) },
-   { rx_large_packets, IBMVETH_STAT_OFF(rx_large_packets) }
+   { rx_large_packets, IBMVETH_STAT_OFF(rx_large_packets) },
+   { fw_enabled_large_send, IBMVETH_STAT_OFF(fw_large_send_support) }
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -848,25 +854,91 @@ static int ibmveth_set_csum_offload(struct net_device 
*dev, u32 data)
return rc1 ? rc1 : rc2;
 }
 
+static int ibmveth_set_tso(struct net_device *dev, u32 data)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   unsigned long set_attr, clr_attr, ret_attr;
+   long ret1, ret2;
+   int rc1 = 0, rc2 = 0;
+   int restart = 0;
+
+   if (netif_running(dev)) {
+   restart = 1;
+   adapter-pool_config = 1;
+   ibmveth_close(dev);
+   adapter-pool_config = 0;
+   }
+
+   set_attr = 0;
+   clr_attr = 0;
+
+   if (data)
+   set_attr = IBMVETH_ILLAN_LRG_SR_ENABLED;
+   else
+   clr_attr = IBMVETH_ILLAN_LRG_SR_ENABLED;
+
+   ret1 = h_illan_attributes(adapter-vdev-unit_address, 0, 0, ret_attr);
+
+   if (ret1 == H_SUCCESS  (ret_attr  IBMVETH_ILLAN_LRG_SND_SUPPORT) 
+   !old_large_send) {
+   ret2 = h_illan_attributes(adapter-vdev-unit_address, clr_attr,
+ set_attr, ret_attr);
+
+   if (ret2 != H_SUCCESS) {
+   netdev_err(dev, unable to change tso settings. %d 
rc=%ld\n,
+  data, ret2);
+
+   h_illan_attributes(adapter-vdev-unit_address,
+  set_attr, clr_attr, ret_attr);
+
+   if (data == 1)
+   dev-features = ~(NETIF_F_TSO | NETIF_F_TSO6);
+   rc1 = -EIO;
+
+   } else {
+   adapter-fw_large_send_support = data;
+   adapter-large_send = data;
+   }
+   } else {
+   /* Older firmware version of large send offload does not
+* support tcp6/ipv6
+*/
+   if (data == 1) {
+   dev-features = ~NETIF_F_TSO6;
+   netdev_info(dev, TSO feature requires all partitions 
to have updated driver);
+   }
+   adapter-large_send = data;
+   }
+
+   if (restart)
+   rc2 = ibmveth_open(dev);
+
+   return rc1 ? rc1 : rc2;
+}
+
 static int ibmveth_set_features(struct net_device *dev,
netdev_features_t features)
 {
struct ibmveth_adapter *adapter = netdev_priv(dev);
int rx_csum = !!(features  NETIF_F_RXCSUM);
-   int rc;
-   netdev_features_t changed = features ^ dev-features;
-
-   if (features  NETIF_F_TSO  changed)
-   netdev_info(dev, TSO feature requires all partitions to have 
updated driver);
+   int large_send = !!(features  (NETIF_F_TSO | NETIF_F_TSO6));
+   int rc1 = 0, rc2 = 0;
 
-   if (rx_csum == adapter-rx_csum)
-   return 0;
+   if (rx_csum != adapter-rx_csum) {
+   rc1 = ibmveth_set_csum_offload(dev, rx_csum);
+   if (rc1  !adapter-rx_csum)
+   dev-features =
+   features  ~(NETIF_F_ALL_CSUM | NETIF_F_RXCSUM);
+   }
 
-   rc = ibmveth_set_csum_offload(dev, rx_csum);
-   if (rc  !adapter-rx_csum)
-   dev-features = features  ~(NETIF_F_ALL_CSUM | NETIF_F_RXCSUM);
+   if (large_send != adapter-large_send) {
+   rc2

Re: [PATCH net-next] Driver for IBM System i/p VNIC protocol

[Thomas Falcon]

On 12/05/2015 09:25 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Fri,  4 Dec 2015 11:49:46 -0600
>
>> +static int ibmvnic_buffs_per_pool = IBMVNIC_BUFFS_PER_POOL;
>> +module_param(ibmvnic_buffs_per_pool, int, S_IRUGO);
>> +MODULE_PARM_DESC(ibmvnic_buffs_per_pool,
>> + "IBMVNIC number of buffers per rx pool");
> Please do not use module parameters for settings that effect
> the basic operation of the driver.

Hi. Thanks for you feedback.  Could you clarify on what constitutes basic 
operation?  I just want to be sure if we should remove the other module 
parameters or just this one in particular?

>
> Instead use facilities that are generic and provide the same
> user interface for all networking drivers, such as ethtool.

Is it possible to change driver settings like this with ethtool? Would adding 
an interface in sysfs to change this setting be acceptable?

>
> Otherwise users would need to learn a different way to configure the
> same thing on different devices, which is not acceptable.
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH net-next v2] Driver for IBM System i/p VNIC protocol

[Thomas Falcon]

On 12/11/2015 06:53 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Tue,  8 Dec 2015 11:52:19 -0600
>
>> +static long h_reg_sub_crq(unsigned long unit_address, unsigned long token,
>> +  unsigned long length, unsigned long *number,
>> +  unsigned long *irq)
>> +{
>> +long rc;
>> +unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> Please declare local variables from longest to shortest line, otherwise
> known as "reverse christmas tree" order.
>
> Audit this in your entire driver.
>
>> +pool->rx_buff = kcalloc(pool->size, sizeof(struct ibmvnic_rx_buff),
>> +GFP_KERNEL);
> Allocation failures not checked until much later in this function, where
> several other resources have been allocated meanwhile.  That doesn't
> make any sense at all.
>
>> +adapter->closing = 1;
> Please use type 'bool' and values 'true' and 'false' for boolean
> values.
>
> Audit this in your entire driver.
>
>> +if (ip_hdr(skb)->version == 4)
>> +tx_crq.v1.flags1 |= IBMVNIC_TX_PROT_IPV4;
>> +else if (ip_hdr(skb)->version == 6)
>> +tx_crq.v1.flags1 |= IBMVNIC_TX_PROT_IPV6;
>> +
> You cannot dereference the protocol header of the SKB without
> first checking the skb->protocol value, otherwise you're looking
> at garbage.
>
>> +static int ibmvnic_set_mac(struct net_device *netdev, void *p)
>> +{
>> +struct ibmvnic_adapter *adapter = netdev_priv(netdev);
>> +struct sockaddr *addr = p;
>> +union ibmvnic_crq crq;
>> +
>> +if (!is_valid_ether_addr(addr->sa_data))
>> +return -EADDRNOTAVAIL;
>> +
>> +memset(, 0, sizeof(crq));
>> +crq.change_mac_addr.first = IBMVNIC_CRQ_CMD;
>> +crq.change_mac_addr.cmd = CHANGE_MAC_ADDR;
>> +ether_addr_copy(_mac_addr.mac_addr[0], addr->sa_data);
>> +ibmvnic_send_crq(adapter, );
>> +
>> +return 0;
>> +}
> You are responsible for copying the new MAC address into dev->dev_addr
> on success.

We do this in another function (handle_change_mac_rsp), which handles the 
response from firmware indicating whether the CHANGE_MAC_ADDR command was 
successful.  Is this acceptable? 

Thank you for your review comments and your time.

>
>> +static int ibmvnic_mii_ioctl(struct net_device *netdev, struct ifreq *ifr,
>> + int cmd)
>> +{
>  ...
>> +return 0;
>> +}
>> +
>> +static int ibmvnic_ioctl(struct net_device *netdev, struct ifreq *ifr, int 
>> cmd)
>> +{
>> +switch (cmd) {
>> +case SIOCGMIIPHY:
>> +case SIOCGMIIREG:
>> +case SIOCSMIIREG:
>> +return ibmvnic_mii_ioctl(netdev, ifr, cmd);
>> +default:
>> +return -EOPNOTSUPP;
>> +}
>> +}
> This really doesn't make any sense.  Please just delete this.  You
> don't support MII reads or writes because they logically don't make
> sense on this device.
>
>> +static struct net_device_stats *ibmvnic_get_stats(struct net_device *dev)
>> +{
>> +struct ibmvnic_adapter *adapter = netdev_priv(dev);
>> +
>> +/* only return the current stats */
>> +return >net_stats;
>> +}
> The default method does this for you as long as you properly use
> net_device's embedded stats, therefore you don't need to provide this
> at all.
>
> That's all I have any energy for, and as you can see nobody else wants
> to even try to review this driver.
>
> It's going to take a lot of respins and time before this driver is
> ready for upstream inclusion.
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] ibmvnic: fix to use list_for_each_safe() when delete items

[Thomas Falcon]

On 06/17/2016 09:53 PM, weiyj...@163.com wrote:
> From: Wei Yongjun <yongjun_...@trendmicro.com.cn>
>
> Since we will remove items off the list using list_del() we need
> to use a safe version of the list_for_each() macro aptly named
> list_for_each_safe().
>
> Signed-off-by: Wei Yongjun <yongjun_...@trendmicro.com.cn>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 864cb21..0b6a922 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3141,14 +3141,14 @@ static void handle_request_ras_comp_num_rsp(union 
> ibmvnic_crq *crq,
>  
>  static void ibmvnic_free_inflight(struct ibmvnic_adapter *adapter)
>  {
> - struct ibmvnic_inflight_cmd *inflight_cmd;
> + struct ibmvnic_inflight_cmd *inflight_cmd, *tmp1;
>   struct device *dev = >vdev->dev;
> - struct ibmvnic_error_buff *error_buff;
> + struct ibmvnic_error_buff *error_buff, *tmp2;
>   unsigned long flags;
>   unsigned long flags2;
>  
>   spin_lock_irqsave(>inflight_lock, flags);
> - list_for_each_entry(inflight_cmd, >inflight, list) {
> + list_for_each_entry_safe(inflight_cmd, tmp1, >inflight, list) {
>   switch (inflight_cmd->crq.generic.cmd) {
>   case LOGIN:
>   dma_unmap_single(dev, adapter->login_buf_token,
> @@ -3165,8 +3165,8 @@ static void ibmvnic_free_inflight(struct 
> ibmvnic_adapter *adapter)
>   break;
>   case REQUEST_ERROR_INFO:
>   spin_lock_irqsave(>error_list_lock, flags2);
> - list_for_each_entry(error_buff, >errors,
> - list) {
> + list_for_each_entry_safe(error_buff, tmp2,
> +  >errors, list) {
>   dma_unmap_single(dev, error_buff->dma,
>error_buff->len,
>DMA_FROM_DEVICE);
>
Thanks!

Acked-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
>
>
>
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH] ibmvnic: fix to use list_for_each_safe() when delete items

[Thomas Falcon]

On 06/20/2016 10:50 AM, Thomas Falcon wrote:
> On 06/17/2016 09:53 PM, weiyj...@163.com wrote:
>> From: Wei Yongjun <yongjun_...@trendmicro.com.cn>
>>
>> Since we will remove items off the list using list_del() we need
>> to use a safe version of the list_for_each() macro aptly named
>> list_for_each_safe().
>>
>> Signed-off-by: Wei Yongjun <yongjun_...@trendmicro.com.cn>
>> ---
>>  drivers/net/ethernet/ibm/ibmvnic.c | 10 +-
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
>> b/drivers/net/ethernet/ibm/ibmvnic.c
>> index 864cb21..0b6a922 100644
>> --- a/drivers/net/ethernet/ibm/ibmvnic.c
>> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
>> @@ -3141,14 +3141,14 @@ static void handle_request_ras_comp_num_rsp(union 
>> ibmvnic_crq *crq,
>>  
>>  static void ibmvnic_free_inflight(struct ibmvnic_adapter *adapter)
>>  {
>> -struct ibmvnic_inflight_cmd *inflight_cmd;
>> +struct ibmvnic_inflight_cmd *inflight_cmd, *tmp1;
>>  struct device *dev = >vdev->dev;
>> -struct ibmvnic_error_buff *error_buff;
>> +struct ibmvnic_error_buff *error_buff, *tmp2;
>>  unsigned long flags;
>>  unsigned long flags2;
>>  
>>  spin_lock_irqsave(>inflight_lock, flags);
>> -list_for_each_entry(inflight_cmd, >inflight, list) {
>> +list_for_each_entry_safe(inflight_cmd, tmp1, >inflight, list) {
>>  switch (inflight_cmd->crq.generic.cmd) {
>>  case LOGIN:
>>  dma_unmap_single(dev, adapter->login_buf_token,
>> @@ -3165,8 +3165,8 @@ static void ibmvnic_free_inflight(struct 
>> ibmvnic_adapter *adapter)
>>  break;
>>  case REQUEST_ERROR_INFO:
>>  spin_lock_irqsave(>error_list_lock, flags2);
>> -list_for_each_entry(error_buff, >errors,
>> -list) {
>> +list_for_each_entry_safe(error_buff, tmp2,
>> + >errors, list) {
>>  dma_unmap_single(dev, error_buff->dma,
>>   error_buff->len,
>>   DMA_FROM_DEVICE);
>>
> Thanks!
>
> Acked-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>

Hello, I apologize for prematurely ack'ing this.  There is another situation 
where you could use list_for_each_entry_safe in the function 
handle_error_info_rsp.  Could you include this in your patch, please?

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 864cb21..e9968d9 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2121,7 +2121,7 @@ static void handle_error_info_rsp(union ibmvnic_crq *crq,
  struct ibmvnic_adapter *adapter)
 {
struct device *dev = >vdev->dev;
-   struct ibmvnic_error_buff *error_buff;
+   struct ibmvnic_error_buff *error_buff, *tmp;
unsigned long flags;
bool found = false;
int i;
@@ -2133,7 +2133,7 @@ static void handle_error_info_rsp(union ibmvnic_crq *crq,
}
 
spin_lock_irqsave(>error_list_lock, flags);
-   list_for_each_entry(error_buff, >errors, list)
+   list_for_each_entry_safe(error_buff, tmp, >errors, list)
if (error_buff->error_id == crq->request_error_rsp.error_id) {
found = true;
list_del(_buff->list);

>>
>>
>> ___
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

[net-next PATCH v2] ibmvnic: map L2/L3/L4 header descriptors to firmware

[Thomas Falcon]

From: root <r...@ltcalpine2-lp23.aus.stglabs.ibm.com>

Allow the VNIC driver to provide descriptors containing
L2/L3/L4 headers to firmware.  This feature is needed
for greater hardware compatibility and enablement of offloading
technologies for some backing hardware.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
v2: Fixed typo error caught by kbuild test bot
---
 drivers/net/ethernet/ibm/ibmvnic.c | 238 -
 1 file changed, 235 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7d657084..43c1df6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -61,6 +61,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -94,6 +95,7 @@ static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter 
*);
 static int ibmvnic_send_crq(struct ibmvnic_adapter *, union ibmvnic_crq *);
 static int send_subcrq(struct ibmvnic_adapter *adapter, u64 remote_handle,
   union sub_crq *sub_crq);
+static int send_subcrq_indirect(struct ibmvnic_adapter *, u64, u64, u64);
 static irqreturn_t ibmvnic_interrupt_rx(int irq, void *instance);
 static int enable_scrq_irq(struct ibmvnic_adapter *,
   struct ibmvnic_sub_crq_queue *);
@@ -561,12 +563,177 @@ static int ibmvnic_close(struct net_device *netdev)
return 0;
 }
 
+/**
+ * build_hdr_data - creates L2/L3/L4 header data buffer
+ * @hdr_field - bitfield determining needed headers
+ * @skb - socket buffer
+ * @hdr_len - array of header lengths
+ * @tot_len - total length of data
+ *
+ * Reads hdr_field to determine which headers are needed by firmware.
+ * Builds a buffer containing these headers.  Saves individual header
+ * lengths and total buffer length to be used to build descriptors.
+ */
+static unsigned char *build_hdr_data(u8 hdr_field, struct sk_buff *skb,
+int *hdr_len, int *tot_len)
+{
+   unsigned char *hdr_data;
+   unsigned char *hdrs[3];
+   u8 proto = 0;
+   int len = 0;
+   int i;
+
+   if ((hdr_field >> 6) & 1) {
+   hdrs[0] = skb_mac_header(skb);
+   hdr_len[0] = sizeof(struct ethhdr);
+   }
+
+   if ((hdr_field >> 5) & 1) {
+   hdrs[1] = skb_network_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   hdr_len[1] = ip_hdr(skb)->ihl * 4;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   hdr_len[1] = sizeof(struct ipv6hdr);
+   }
+
+   if ((hdr_field >> 4) & 1) {
+   hdrs[2] = skb_transport_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   proto = ip_hdr(skb)->protocol;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   proto = ipv6_hdr(skb)->nexthdr;
+
+   if (proto == IPPROTO_TCP)
+   hdr_len[2] = tcp_hdrlen(skb);
+   else if (proto == IPPROTO_UDP)
+   hdr_len[2] = sizeof(struct udphdr);
+   }
+
+   *tot_len = hdr_len[0] + hdr_len[1] + hdr_len[2];
+
+   hdr_data = kmalloc(*tot_len, GFP_KERNEL);
+   if (!hdr_data)
+   return NULL;
+
+   for (i = 0; i < 3; i++) {
+   if (hdrs[i])
+   memcpy(hdr_data, hdrs[i] + len, hdr_len[i]);
+   len += hdr_len[i];
+   }
+   return hdr_data;
+}
+
+/**
+ * create_hdr_descs - create header and header extension descriptors
+ * @hdr_field - bitfield determining needed headers
+ * @data - buffer containing header data
+ * @len - length of data buffer
+ * @hdr_len - array of individual header lengths
+ * @scrq_arr - descriptor array
+ *
+ * Creates header and, if needed, header extension descriptors and
+ * places them in a descriptor array, scrq_arr
+ */
+
+void create_hdr_descs(u8 hdr_field, unsigned char *data, int len, int *hdr_len,
+ union sub_crq *scrq_arr)
+{
+   union sub_crq hdr_desc;
+   int tmp_len = len;
+   int tmp;
+
+   while (tmp_len > 0) {
+   unsigned char *cur = data + len - tmp_len;
+
+   memset(_desc, 0, sizeof(hdr_desc));
+   if (cur != data) {
+   tmp = tmp_len > 29 ? 29 : tmp_len;
+   hdr_desc.hdr_ext.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr_ext.type = IBMVNIC_HDR_EXT_DESC;
+   hdr_desc.hdr_ext.len = tmp;
+   } else {
+   tmp = tmp_len > 24 ? 24 : tmp_len;
+   hdr_desc.hdr.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr.type = IBMVNIC_HDR_DESC;
+   hdr_desc.hdr.len = tmp;
+   hdr_desc.hdr.l2_len = (

[net-next PATCH v3] ibmvnic: map L2/L3/L4 header descriptors to firmware

[Thomas Falcon]

Allow the VNIC driver to provide descriptors containing
L2/L3/L4 headers to firmware.  This feature is needed
for greater hardware compatibility and enablement of offloading
technologies for some backing hardware.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
v2: Fixed typo error caught by kbuild test bot
v3: Fixed erroneous patch sender
---
 drivers/net/ethernet/ibm/ibmvnic.c | 238 -
 1 file changed, 235 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7d657084..43c1df6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -61,6 +61,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -94,6 +95,7 @@ static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter 
*);
 static int ibmvnic_send_crq(struct ibmvnic_adapter *, union ibmvnic_crq *);
 static int send_subcrq(struct ibmvnic_adapter *adapter, u64 remote_handle,
   union sub_crq *sub_crq);
+static int send_subcrq_indirect(struct ibmvnic_adapter *, u64, u64, u64);
 static irqreturn_t ibmvnic_interrupt_rx(int irq, void *instance);
 static int enable_scrq_irq(struct ibmvnic_adapter *,
   struct ibmvnic_sub_crq_queue *);
@@ -561,12 +563,177 @@ static int ibmvnic_close(struct net_device *netdev)
return 0;
 }
 
+/**
+ * build_hdr_data - creates L2/L3/L4 header data buffer
+ * @hdr_field - bitfield determining needed headers
+ * @skb - socket buffer
+ * @hdr_len - array of header lengths
+ * @tot_len - total length of data
+ *
+ * Reads hdr_field to determine which headers are needed by firmware.
+ * Builds a buffer containing these headers.  Saves individual header
+ * lengths and total buffer length to be used to build descriptors.
+ */
+static unsigned char *build_hdr_data(u8 hdr_field, struct sk_buff *skb,
+int *hdr_len, int *tot_len)
+{
+   unsigned char *hdr_data;
+   unsigned char *hdrs[3];
+   u8 proto = 0;
+   int len = 0;
+   int i;
+
+   if ((hdr_field >> 6) & 1) {
+   hdrs[0] = skb_mac_header(skb);
+   hdr_len[0] = sizeof(struct ethhdr);
+   }
+
+   if ((hdr_field >> 5) & 1) {
+   hdrs[1] = skb_network_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   hdr_len[1] = ip_hdr(skb)->ihl * 4;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   hdr_len[1] = sizeof(struct ipv6hdr);
+   }
+
+   if ((hdr_field >> 4) & 1) {
+   hdrs[2] = skb_transport_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   proto = ip_hdr(skb)->protocol;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   proto = ipv6_hdr(skb)->nexthdr;
+
+   if (proto == IPPROTO_TCP)
+   hdr_len[2] = tcp_hdrlen(skb);
+   else if (proto == IPPROTO_UDP)
+   hdr_len[2] = sizeof(struct udphdr);
+   }
+
+   *tot_len = hdr_len[0] + hdr_len[1] + hdr_len[2];
+
+   hdr_data = kmalloc(*tot_len, GFP_KERNEL);
+   if (!hdr_data)
+   return NULL;
+
+   for (i = 0; i < 3; i++) {
+   if (hdrs[i])
+   memcpy(hdr_data, hdrs[i] + len, hdr_len[i]);
+   len += hdr_len[i];
+   }
+   return hdr_data;
+}
+
+/**
+ * create_hdr_descs - create header and header extension descriptors
+ * @hdr_field - bitfield determining needed headers
+ * @data - buffer containing header data
+ * @len - length of data buffer
+ * @hdr_len - array of individual header lengths
+ * @scrq_arr - descriptor array
+ *
+ * Creates header and, if needed, header extension descriptors and
+ * places them in a descriptor array, scrq_arr
+ */
+
+void create_hdr_descs(u8 hdr_field, unsigned char *data, int len, int *hdr_len,
+ union sub_crq *scrq_arr)
+{
+   union sub_crq hdr_desc;
+   int tmp_len = len;
+   int tmp;
+
+   while (tmp_len > 0) {
+   unsigned char *cur = data + len - tmp_len;
+
+   memset(_desc, 0, sizeof(hdr_desc));
+   if (cur != data) {
+   tmp = tmp_len > 29 ? 29 : tmp_len;
+   hdr_desc.hdr_ext.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr_ext.type = IBMVNIC_HDR_EXT_DESC;
+   hdr_desc.hdr_ext.len = tmp;
+   } else {
+   tmp = tmp_len > 24 ? 24 : tmp_len;
+   hdr_desc.hdr.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr.type = IBMVNIC_HDR_DESC;
+   hdr_desc.hdr.len = tmp;
+   hdr_desc.hdr.l2_len = (u8)hdr_len[0];
+   hdr_desc.hdr.l3_len = (u16)hd

[net-next PATCH] ibmvnic: map L2/L3/L4 header descriptors to firmware

[Thomas Falcon]

Allow the VNIC driver to provide descriptors containing
L2/L3/L4 headers to firmware.  This feature is needed
for greater hardware compatibility and enablement of offloading
technologies for some backing hardware.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 238 -
 1 file changed, 235 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7d657084..43c1df6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -61,6 +61,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -94,6 +95,7 @@ static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter 
*);
 static int ibmvnic_send_crq(struct ibmvnic_adapter *, union ibmvnic_crq *);
 static int send_subcrq(struct ibmvnic_adapter *adapter, u64 remote_handle,
   union sub_crq *sub_crq);
+static int send_subcrq_indirect(struct ibmvnic_adapter *, u64, u64, u64);
 static irqreturn_t ibmvnic_interrupt_rx(int irq, void *instance);
 static int enable_scrq_irq(struct ibmvnic_adapter *,
   struct ibmvnic_sub_crq_queue *);
@@ -561,12 +563,177 @@ static int ibmvnic_close(struct net_device *netdev)
return 0;
 }
 
+/**
+ * build_hdr_data - creates L2/L3/L4 header data buffer
+ * @hdr_field - bitfield determining needed headers
+ * @skb - socket buffer
+ * @hdr_len - array of header lengths
+ * @tot_len - total length of data
+ *
+ * Reads hdr_field to determine which headers are needed by firmware.
+ * Builds a buffer containing these headers.  Saves individual header
+ * lengths and total buffer length to be used to build descriptors.
+ */
+static unsigned char *build_hdr_data(u8 hdr_field, struct sk_buff *skb,
+int *hdr_len, int *tot_len)
+{
+   unsigned char *hdr_data;
+   unsigned char *hdrs[3];
+   u8 proto = 0;
+   int len = 0;
+   int i;
+
+   if ((hdr_field >> 6) & 1) {
+   hdrs[0] = skb_mac_header(skb);
+   hdr_len[0] = sizeof(struct ethhdr);
+   }
+
+   if ((hdr_field >> 5) & 1) {
+   hdrs[1] = skb_network_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   hdr_len[1] = ip_hdr(skb)->ihl * 4;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   hdr_len[1] = sizeof(struct ipv6hdr);
+   }
+
+   if ((hdr_field >> 4) & 1) {
+   hdrs[2] = skb_transport_header(skb);
+   if (skb->protocol == htons(ETH_P_IP))
+   proto = ip_hdr(skb)->protocol;
+   else if (skb->protocol == htons(ETH_P_IPV6))
+   proto = ipv6_hdr(skb)->nexthdr;
+
+   if (proto == IPPROTO_TCP)
+   hdr_len[2] = tcp_hdrlen(skb);
+   else if (proto == IPPROTO_UDP)
+   hdr_len[2] = sizeof(struct udphdr);
+   }
+
+   *tot_len = hdr_len[0] + hdr_len[1] + hdr_len[2];
+
+   hdr_data = kmalloc(*tot_len, GFP_KERNEL);
+   if (!hdr_data)
+   return NULL;
+
+   for (i = 0; i < 3; i++) {
+   if (hdrs[i])
+   memcpy(hdr_data, hdrs[i] + len, hdr_len[i]);
+   len += hdr_len[i];
+   }
+   return hdr_data;
+}
+
+/**
+ * create_hdr_descs - create header and header extension descriptors
+ * @hdr_field - bitfield determining needed headers
+ * @data - buffer containing header data
+ * @len - length of data buffer
+ * @hdr_len - array of individual header lengths
+ * @scrq_arr - descriptor array
+ *
+ * Creates header and, if needed, header extension descriptors and
+ * places them in a descriptor array, scrq_arr
+ */
+
+void create_hdr_descs(u8 hdr_field, unsigned char *data, int len, int *hdr_len,
+ union sub_crq *scrq_arr)
+{
+   union sub_crq hdr_desc;
+   int tmp_len = len;
+   int tmp;
+
+   while (tmp_len > 0) {
+   unsigned char *cur = data + len - tmp_len;
+
+   memset(_desc, 0, sizeof(hdr_desc));
+   if (cur != data) {
+   tmp = tmp_len > 29 ? 29 : tmp_len;
+   hdr_desc.hdr_ext.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr_ext.type = IBMVNIC_HDR_EXT_DESC;
+   hdr_desc.hdr_ext.len = tmp;
+   } else {
+   tmp = tmp_len > 24 ? 24 : tmp_len;
+   hdr_desc.hdr.first = IBMVNIC_CRQ_CMD;
+   hdr_desc.hdr.type = IBMVNIC_HDR_DESC;
+   hdr_desc.hdr.len = tmp;
+   hdr_desc.hdr.l2_len = (u8)hdr_len[0];
+   hdr_desc.hdr.l3_len = (u16)hdr_len[1];
+   hdr_desc.hdr.l4_len = (u8)h

[net-next PATCH 1/2 v4] ibmvnic: map L2/L3/L4 header descriptors to firmware

[Thomas Falcon]

Allow the VNIC driver to provide descriptors containing
L2/L3/L4 headers to firmware.  This feature is needed
for greater hardware compatibility and enablement of checksum
and TCP offloading features.

A new function is included for the hypervisor call,
H_SEND_SUBCRQ_INDIRECT, allowing a DMA-mapped array of SCRQ
descriptor elements to be sent to the VNIC server.

These additions will help fully enable checksum offloading as
well as other features as they are included later.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
Cc: John Allen <jal...@linux.vnet.ibm.com>
---
v2: Fixed typo error caught by kbuild test bot
v3: Fixed erroneous patch sender
v4: sorry for the delay in resending, Thanks to David Miller for comments,
removed all extra memory allocations,
merged some helper functions,
calculate all header lengths to meet firmware requirements,
fixed endian bugs in the send_subcrq_indirect
---
 drivers/net/ethernet/ibm/ibmvnic.c | 195 -
 drivers/net/ethernet/ibm/ibmvnic.h |   3 +
 2 files changed, 194 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 6e9e16ee..4e97e76 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -61,6 +61,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -94,6 +95,7 @@ static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter 
*);
 static int ibmvnic_send_crq(struct ibmvnic_adapter *, union ibmvnic_crq *);
 static int send_subcrq(struct ibmvnic_adapter *adapter, u64 remote_handle,
   union sub_crq *sub_crq);
+static int send_subcrq_indirect(struct ibmvnic_adapter *, u64, u64, u64);
 static irqreturn_t ibmvnic_interrupt_rx(int irq, void *instance);
 static int enable_scrq_irq(struct ibmvnic_adapter *,
   struct ibmvnic_sub_crq_queue *);
@@ -561,10 +563,141 @@ static int ibmvnic_close(struct net_device *netdev)
return 0;
 }
 
+/**
+ * build_hdr_data - creates L2/L3/L4 header data buffer
+ * @hdr_field - bitfield determining needed headers
+ * @skb - socket buffer
+ * @hdr_len - array of header lengths
+ * @tot_len - total length of data
+ *
+ * Reads hdr_field to determine which headers are needed by firmware.
+ * Builds a buffer containing these headers.  Saves individual header
+ * lengths and total buffer length to be used to build descriptors.
+ */
+static int build_hdr_data(u8 hdr_field, struct sk_buff *skb,
+ int *hdr_len, u8 *hdr_data)
+{
+   int len = 0;
+   u8 *hdr;
+
+   hdr_len[0] = sizeof(struct ethhdr);
+
+   if (skb->protocol == htons(ETH_P_IP)) {
+   hdr_len[1] = ip_hdr(skb)->ihl * 4;
+   if (ip_hdr(skb)->protocol == IPPROTO_TCP)
+   hdr_len[2] = tcp_hdrlen(skb);
+   else if (ip_hdr(skb)->protocol == IPPROTO_UDP)
+   hdr_len[2] = sizeof(struct udphdr);
+   } else if (skb->protocol == htons(ETH_P_IPV6)) {
+   hdr_len[1] = sizeof(struct ipv6hdr);
+   if (ipv6_hdr(skb)->nexthdr == IPPROTO_TCP)
+   hdr_len[2] = tcp_hdrlen(skb);
+   else if (ipv6_hdr(skb)->nexthdr == IPPROTO_UDP)
+   hdr_len[2] = sizeof(struct udphdr);
+   }
+
+   memset(hdr_data, 0, 120);
+   if ((hdr_field >> 6) & 1) {
+   hdr = skb_mac_header(skb);
+   memcpy(hdr_data, hdr, hdr_len[0]);
+   len += hdr_len[0];
+   }
+
+   if ((hdr_field >> 5) & 1) {
+   hdr = skb_network_header(skb);
+   memcpy(hdr_data + len, hdr, hdr_len[1]);
+   len += hdr_len[1];
+   }
+
+   if ((hdr_field >> 4) & 1) {
+   hdr = skb_transport_header(skb);
+   memcpy(hdr_data + len, hdr, hdr_len[2]);
+   len += hdr_len[2];
+   }
+   return len;
+}
+
+/**
+ * create_hdr_descs - create header and header extension descriptors
+ * @hdr_field - bitfield determining needed headers
+ * @data - buffer containing header data
+ * @len - length of data buffer
+ * @hdr_len - array of individual header lengths
+ * @scrq_arr - descriptor array
+ *
+ * Creates header and, if needed, header extension descriptors and
+ * places them in a descriptor array, scrq_arr
+ */
+
+static void create_hdr_descs(u8 hdr_field, u8 *hdr_data, int len, int *hdr_len,
+union sub_crq *scrq_arr)
+{
+   union sub_crq hdr_desc;
+   int tmp_len = len;
+   u8 *data, *cur;
+   int tmp;
+
+   while (tmp_len > 0) {
+   cur = hdr_data + len - tmp_len;
+
+   memset(_desc, 0, sizeof(hdr_desc));
+   if (cur != hdr_data) {
+   data = hdr_desc.hdr_ext.data;
+   tmp = tmp_len > 29 ? 29 : tmp_

[net-next PATCH 2/2 v4] ibmvnic: enable RX checksum offload

[Thomas Falcon]

Enable RX Checksum offload feature in the ibmvnic driver.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
Cc: John Allen <jal...@linux.vnet.ibm.com>
---
v4: this patch included since it is enabled by the previous patch
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4e97e76..21bccf6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2105,6 +2105,10 @@ static void handle_query_ip_offload_rsp(struct 
ibmvnic_adapter *adapter)
if (buf->tcp_ipv6_chksum || buf->udp_ipv6_chksum)
adapter->netdev->features |= NETIF_F_IPV6_CSUM;
 
+   if ((adapter->netdev->features &
+   (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
+   adapter->netdev->features |= NETIF_F_RXCSUM;
+
memset(, 0, sizeof(crq));
crq.control_ip_offload.first = IBMVNIC_CRQ_CMD;
crq.control_ip_offload.cmd = CONTROL_IP_OFFLOAD;
-- 
2.4.11

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [PATCH v2 1/2] powerpc/pseries: Implemented indexed-count hotplug memory add

[Thomas Falcon]

On 07/18/2016 10:07 AM, Sahil Mehta wrote:
> Indexed-count add for memory hotplug guarantees that a contiguous block
> of  lmbs beginning at a specified  will be assigned (NOT
> that  lmbs will be added). Because of Qemu's per-DIMM memory
> management, the addition of a contiguous block of memory currently
> requires a series of individual calls. Indexed-count add reduces
> this series into a single call.
>
> Signed-off-by: Sahil Mehta 
> ---
> v2:   -remove potential memory leak when parsing command
>   -use u32s drc_index and count instead of u32 ic[]
>in dlpar_memory
>
>  arch/powerpc/include/asm/rtas.h |2
>  arch/powerpc/platforms/pseries/dlpar.c  |   34 +++-
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  100 
> +--
>  3 files changed, 124 insertions(+), 12 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 51400ba..f46b271 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -307,6 +307,7 @@ struct pseries_hp_errorlog {
>   union {
>   __be32  drc_index;
>   __be32  drc_count;
> + __be32  indexed_count[2];
>   chardrc_name[1];
>   } _drc_u;
>  };
> @@ -322,6 +323,7 @@ struct pseries_hp_errorlog {
>  #define PSERIES_HP_ELOG_ID_DRC_NAME  1
>  #define PSERIES_HP_ELOG_ID_DRC_INDEX 2
>  #define PSERIES_HP_ELOG_ID_DRC_COUNT 3
> +#define PSERIES_HP_ELOG_ID_IC4
>
>  struct pseries_errorlog *get_pseries_errorlog(struct rtas_error_log *log,
> uint16_t section_id);
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
> b/arch/powerpc/platforms/pseries/dlpar.c
> index 2b93ae8..2a6dc9e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -345,11 +345,17 @@ static int handle_dlpar_errorlog(struct 
> pseries_hp_errorlog *hp_elog)
>   switch (hp_elog->id_type) {
>   case PSERIES_HP_ELOG_ID_DRC_COUNT:
>   hp_elog->_drc_u.drc_count =
> - be32_to_cpu(hp_elog->_drc_u.drc_count);
> + be32_to_cpu(hp_elog->_drc_u.drc_count);
>   break;
>   case PSERIES_HP_ELOG_ID_DRC_INDEX:
>   hp_elog->_drc_u.drc_index =
> - be32_to_cpu(hp_elog->_drc_u.drc_index);
> + be32_to_cpu(hp_elog->_drc_u.drc_index);
> + break;
> + case PSERIES_HP_ELOG_ID_IC:
> + hp_elog->_drc_u.indexed_count[0] =
> + be32_to_cpu(hp_elog->_drc_u.indexed_count[0]);
> + hp_elog->_drc_u.indexed_count[1] =
> + be32_to_cpu(hp_elog->_drc_u.indexed_count[1]);
>   }
>
>   switch (hp_elog->resource) {
> @@ -409,7 +415,29 @@ static ssize_t dlpar_store(struct class *class, struct 
> class_attribute *attr,
>   goto dlpar_store_out;
>   }
>
> - if (!strncmp(arg, "index", 5)) {
> + if (!strncmp(arg, "indexed-count", 13)) {
> + u32 index, count;
> + char *cstr, *istr;
> +
> + hp_elog->id_type = PSERIES_HP_ELOG_ID_IC;
> + arg += strlen("indexed-count ");
> +
> + cstr = kstrdup(arg, GFP_KERNEL);
> + istr = strchr(cstr, ' ');
> + *istr++ = '\0';
> +
> + if (kstrtou32(cstr, 0, ) || kstrtou32(istr, 0, )) {
> + rc = -EINVAL;
> + pr_err("Invalid index or count : \"%s\"\n", buf);
> + kfree(cstr);
> + goto dlpar_store_out;
> + }
> +
> + kfree(cstr);
> +
> + hp_elog->_drc_u.indexed_count[0] = cpu_to_be32(count);
> + hp_elog->_drc_u.indexed_count[1] = cpu_to_be32(index);
> + } else if (!strncmp(arg, "index", 5)) {
>   u32 index;
>
>   hp_elog->id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 2ce1385..d7942ca 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -701,6 +701,83 @@ static int dlpar_memory_add_by_index(u32 drc_index, 
> struct property *prop)
>   return rc;
>  }
>
> +static int dlpar_memory_add_by_ic(u32 lmbs_to_add, u32 drc_index,
> +   struct property *prop)
> +{
> + struct of_drconf_cell *lmbs;
> + u32 num_lmbs, *p;
> + int i, rc;
> + int lmbs_available = 0, start_index = 0, end_index;
> +
> + pr_info("Attempting to hot-add %u LMB(s) at index %x\n",
> + lmbs_to_add, drc_index);
> +
> + if (lmbs_to_add == 0)
> + return -EINVAL;
> +
> + p = prop->value;
> + num_lmbs = *p++;
> + lmbs = (struct 

Re: [PATCH] ibmveth: Add a proper check for the availability of the checksum features

[Thomas Falcon]

On 01/24/2017 12:28 AM, Thomas Huth wrote:
> When using the ibmveth driver in a KVM/QEMU based VM, it currently
> always prints out a scary error message like this when it is started:
>
>  ibmveth 7103 (unregistered net_device): unable to change
>  checksum offload settings. 1 rc=-2 ret_attr=7103
>
> This happens because the driver always tries to enable the checksum
> offloading without checking for the availability of this feature first.
> QEMU does not support checksum offloading for the spapr-vlan device,
> thus we always get the error message here.
> According to the LoPAPR specification, the "ibm,illan-options" property
> of the corresponding device tree node should be checked first to see
> whether the H_ILLAN_ATTRIUBTES hypercall and thus the checksum offloading
> feature is available. Thus let's do this in the ibmveth driver, too, so
> that the error message is really only limited to cases where something
> goes wrong, and does not occur if the feature is just missing.

Thanks a lot for this patch, Thomas.  Was going to give an Ack, but its already 
been applied :)

>
> Signed-off-by: Thomas Huth 
> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
> b/drivers/net/ethernet/ibm/ibmveth.c
> index a831f94..309f5c6 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -1601,8 +1601,11 @@ static int ibmveth_probe(struct vio_dev *dev, const 
> struct vio_device_id *id)
>   netdev->netdev_ops = _netdev_ops;
>   netdev->ethtool_ops = _ethtool_ops;
>   SET_NETDEV_DEV(netdev, >dev);
> - netdev->hw_features = NETIF_F_SG | NETIF_F_RXCSUM |
> - NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> + netdev->hw_features = NETIF_F_SG;
> + if (vio_get_attribute(dev, "ibm,illan-options", NULL) != NULL) {
> + netdev->hw_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> +NETIF_F_RXCSUM;
> + }
>
>   netdev->features |= netdev->hw_features;
>

Re: [PATCH net 1/5] ibmvnic: harden interrupt handler

[Thomas Falcon]

On 01/25/2017 10:04 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Wed, 25 Jan 2017 15:02:19 -0600
>
>> Move most interrupt handler processing into a tasklet, and
>> introduce a delay after re-enabling interrupts to fix timing
>> issues encountered in hardware testing.
>>
>> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> I don't think you have any idea what the real problem is.  This looks
> like a hack, at best.  Next patch you'll increase the delay to "20",
> right?  And if that doesn't work you'll try "40".
>
> Or if you do know the reason, you need to explain it in detail in this
> commit message so that we can properly evaluate this patch.

You're right, I should have given more explanation in the commit message about 
the bug encountered and our reasons for this sort of fix.  The issue is that 
there are some scenarios during the device init where multiple messages are 
sent by firmware in one interrupt request. 

We have observed behavior where messages are delayed, resulting in the 
interrupt handler completing before the delayed messages can be processed, 
fouling up the device bring-up in the device probing and elsewhere.  The goal 
of the delay is to buy some time for the hypervisor to forward all the CRQ 
messages from the firmware.
>
> Furthermore, if you're going to move all of your packet processing
> into software interrupt context, you might as well use NAPI polling
> which is a purposefully built piece of infrastructure for doing
> exactly this.
>
This interrupt handler doesn't handle packet processing, but communications 
between the driver and firmware for device settings and resource allocation.  
Packet processing is done with different interrupts that do use NAPI polling.

Re: [PATCH net 2/5] ibmvnic: Fix MTU settings

[Thomas Falcon]

On 01/25/2017 10:05 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Wed, 25 Jan 2017 15:02:20 -0600
>
>> In the current driver, the MTU is set to the maximum value
>> capable for the backing device. This patch sets the MTU to the
>> default value for a Linux net device.
> Why are you doing this?
>
> What happens to users who depend upon the current behavior.
>
> They will break, and that isn't acceptable.
>
The current behavior was already broken.  We were seeing firmware errors when 
sending jumbo sized packets .  We plan to add proper support for jumbo sized 
packets as soon as possible.  

Re: [PATCH net 1/5] ibmvnic: harden interrupt handler

[Thomas Falcon]

On 01/26/2017 11:56 AM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Thu, 26 Jan 2017 10:44:22 -0600
>
>> On 01/25/2017 10:04 PM, David Miller wrote:
>>> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
>>> Date: Wed, 25 Jan 2017 15:02:19 -0600
>>>
>>>> Move most interrupt handler processing into a tasklet, and
>>>> introduce a delay after re-enabling interrupts to fix timing
>>>> issues encountered in hardware testing.
>>>>
>>>> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
>>> I don't think you have any idea what the real problem is.  This looks
>>> like a hack, at best.  Next patch you'll increase the delay to "20",
>>> right?  And if that doesn't work you'll try "40".
>>>
>>> Or if you do know the reason, you need to explain it in detail in this
>>> commit message so that we can properly evaluate this patch.
>> You're right, I should have given more explanation in the commit message 
>> about the bug encountered and our reasons for this sort of fix.  The issue 
>> is that there are some scenarios during the device init where multiple 
>> messages are sent by firmware in one interrupt request. 
>>
>> We have observed behavior where messages are delayed, resulting in the 
>> interrupt handler completing before the delayed messages can be processed, 
>> fouling up the device bring-up in the device probing and elsewhere.  The 
>> goal of the delay is to buy some time for the hypervisor to forward all the 
>> CRQ messages from the firmware.
> Then isn't there an event you can sleep and wait for, or a piece of state for
> you to poll and test for in a timeout based loop?
>
> This delay is a bad solution for the problem of waiting for A to happen
> before you do B.
>
Understood.  We will come up with a better fix.  Thanks for your attention.

Re: [PATCH net 1/5] ibmvnic: harden interrupt handler

[Thomas Falcon]

On 01/26/2017 12:28 PM, Stephen Hemminger wrote:
> On Wed, 25 Jan 2017 15:02:19 -0600
> Thomas Falcon <tlfal...@linux.vnet.ibm.com> wrote:
>
>>  static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
>>  {
>>  struct ibmvnic_adapter *adapter = instance;
>> +unsigned long flags;
>> +
>> +spin_lock_irqsave(>crq.lock, flags);
>> +vio_disable_interrupts(adapter->vdev);
>> +tasklet_schedule(>tasklet);
>> +spin_unlock_irqrestore(>crq.lock, flags);
>> +return IRQ_HANDLED;
>> +}
>> +
> Why not use NAPI? rather than a tasklet
>
This interrupt function doesn't process packets, but message passing between 
firmware and driver for determining device capabilities and available 
resources, such as the number TX and RX queues. 

[PATCH net 3/5] ibmvnic: Fix endian error when requesting device capabilites

[Thomas Falcon]

When a IBM VNIC client driver requests a faulty device setting, the
server returns an acceptable value for the client to request.
This 64 bit value was incorrectly being swapped as a 32 bit value,
resulting in loss of data. This patch corrects that by using
the 64 bit swap function.

Signed-off-by: John Allen <jal...@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index f95f6a4..ec4eaed 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2388,10 +2388,10 @@ static void handle_request_cap_rsp(union ibmvnic_crq 
*crq,
case PARTIALSUCCESS:
dev_info(dev, "req=%lld, rsp=%ld in %s queue, retrying.\n",
 *req_value,
-(long int)be32_to_cpu(crq->request_capability_rsp.
+(long int)be64_to_cpu(crq->request_capability_rsp.
   number), name);
release_sub_crqs_no_irqs(adapter);
-   *req_value = be32_to_cpu(crq->request_capability_rsp.number);
+   *req_value = be64_to_cpu(crq->request_capability_rsp.number);
init_sub_crqs(adapter, 1);
return;
default:
-- 
1.8.3.1

[PATCH net 1/5] ibmvnic: harden interrupt handler

[Thomas Falcon]

Move most interrupt handler processing into a tasklet, and
introduce a delay after re-enabling interrupts to fix timing
issues encountered in hardware testing.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 21 +++--
 drivers/net/ethernet/ibm/ibmvnic.h |  1 +
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index c125966..09071bf 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3414,6 +3414,18 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
 static irqreturn_t ibmvnic_interrupt(int irq, void *instance)
 {
struct ibmvnic_adapter *adapter = instance;
+   unsigned long flags;
+
+   spin_lock_irqsave(>crq.lock, flags);
+   vio_disable_interrupts(adapter->vdev);
+   tasklet_schedule(>tasklet);
+   spin_unlock_irqrestore(>crq.lock, flags);
+   return IRQ_HANDLED;
+}
+
+static void ibmvnic_tasklet(void *data)
+{
+   struct ibmvnic_adapter *adapter = data;
struct ibmvnic_crq_queue *queue = >crq;
struct vio_dev *vdev = adapter->vdev;
union ibmvnic_crq *crq;
@@ -3421,7 +3433,6 @@ static irqreturn_t ibmvnic_interrupt(int irq, void 
*instance)
bool done = false;
 
spin_lock_irqsave(>lock, flags);
-   vio_disable_interrupts(vdev);
while (!done) {
/* Pull all the valid messages off the CRQ */
while ((crq = ibmvnic_next_crq(adapter)) != NULL) {
@@ -3429,6 +3440,8 @@ static irqreturn_t ibmvnic_interrupt(int irq, void 
*instance)
crq->generic.first = 0;
}
vio_enable_interrupts(vdev);
+   /* delay in case of firmware hiccup */
+   mdelay(10);
crq = ibmvnic_next_crq(adapter);
if (crq) {
vio_disable_interrupts(vdev);
@@ -3439,7 +3452,6 @@ static irqreturn_t ibmvnic_interrupt(int irq, void 
*instance)
}
}
spin_unlock_irqrestore(>lock, flags);
-   return IRQ_HANDLED;
 }
 
 static int ibmvnic_reenable_crq_queue(struct ibmvnic_adapter *adapter)
@@ -3494,6 +3506,7 @@ static void ibmvnic_release_crq_queue(struct 
ibmvnic_adapter *adapter)
 
netdev_dbg(adapter->netdev, "Releasing CRQ\n");
free_irq(vdev->irq, adapter);
+   tasklet_kill(>tasklet);
do {
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
@@ -3539,6 +3552,9 @@ static int ibmvnic_init_crq_queue(struct ibmvnic_adapter 
*adapter)
 
retrc = 0;
 
+   tasklet_init(>tasklet, (void *)ibmvnic_tasklet,
+(unsigned long)adapter);
+
netdev_dbg(adapter->netdev, "registering irq 0x%x\n", vdev->irq);
rc = request_irq(vdev->irq, ibmvnic_interrupt, 0, IBMVNIC_NAME,
 adapter);
@@ -3560,6 +3576,7 @@ static int ibmvnic_init_crq_queue(struct ibmvnic_adapter 
*adapter)
return retrc;
 
 req_irq_failed:
+   tasklet_kill(>tasklet);
do {
rc = plpar_hcall_norets(H_FREE_CRQ, vdev->unit_address);
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h 
b/drivers/net/ethernet/ibm/ibmvnic.h
index dd775d9..0d0edc3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -1049,5 +1049,6 @@ struct ibmvnic_adapter {
 
struct work_struct vnic_crq_init;
struct work_struct ibmvnic_xport;
+   struct tasklet_struct tasklet;
bool failover;
 };
-- 
2.7.4

[PATCH net 2/5] ibmvnic: Fix MTU settings

[Thomas Falcon]

In the current driver, the MTU is set to the maximum value
capable for the backing device. This patch sets the MTU to the
default value for a Linux net device. It also corrects a
discrepancy between MTU values received from firmware, which includes
the ethernet header length, and net device MTU values. Finally, it removes
redundant min/max MTU assignments after device initialization.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 48debde2..f95f6a4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1496,7 +1496,7 @@ static void init_sub_crqs(struct ibmvnic_adapter 
*adapter, int retry)
adapter->req_rx_queues = adapter->opt_rx_comp_queues;
adapter->req_rx_add_queues = adapter->max_rx_add_queues;
 
-   adapter->req_mtu = adapter->max_mtu;
+   adapter->req_mtu = adapter->netdev->mtu + ETH_HLEN;
}
 
total_queues = adapter->req_tx_queues + adapter->req_rx_queues;
@@ -2626,12 +2626,12 @@ static void handle_query_cap_rsp(union ibmvnic_crq *crq,
break;
case MIN_MTU:
adapter->min_mtu = be64_to_cpu(crq->query_capability.number);
-   netdev->min_mtu = adapter->min_mtu;
+   netdev->min_mtu = adapter->min_mtu - ETH_HLEN;
netdev_dbg(netdev, "min_mtu = %lld\n", adapter->min_mtu);
break;
case MAX_MTU:
adapter->max_mtu = be64_to_cpu(crq->query_capability.number);
-   netdev->max_mtu = adapter->max_mtu;
+   netdev->max_mtu = adapter->max_mtu - ETH_HLEN;
netdev_dbg(netdev, "max_mtu = %lld\n", adapter->max_mtu);
break;
case MAX_MULTICAST_FILTERS:
@@ -3672,9 +3672,7 @@ static void handle_crq_init_rsp(struct work_struct *work)
goto task_failed;
 
netdev->real_num_tx_queues = adapter->req_tx_queues;
-   netdev->mtu = adapter->req_mtu;
-   netdev->min_mtu = adapter->min_mtu;
-   netdev->max_mtu = adapter->max_mtu;
+   netdev->mtu = adapter->req_mtu - ETH_HLEN;
 
if (adapter->failover) {
adapter->failover = false;
@@ -3814,7 +3812,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
}
 
netdev->real_num_tx_queues = adapter->req_tx_queues;
-   netdev->mtu = adapter->req_mtu;
+   netdev->mtu = adapter->req_mtu - ETH_HLEN;
 
rc = register_netdev(netdev);
if (rc) {
-- 
1.8.3.1

[PATCH net 4/5] ibmvnic: Fix endian errors in error reporting output

[Thomas Falcon]

Error reports received from firmware were not being converted from
big endian values, leading to bogus error codes reported on little
endian systems.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index ec4eaed..ec6c5fe 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2185,12 +2185,12 @@ static void handle_error_info_rsp(union ibmvnic_crq 
*crq,
 
if (!found) {
dev_err(dev, "Couldn't find error id %x\n",
-   crq->request_error_rsp.error_id);
+   be32_to_cpu(crq->request_error_rsp.error_id));
return;
}
 
dev_err(dev, "Detailed info for error id %x:",
-   crq->request_error_rsp.error_id);
+   be32_to_cpu(crq->request_error_rsp.error_id));
 
for (i = 0; i < error_buff->len; i++) {
pr_cont("%02x", (int)error_buff->buff[i]);
@@ -2269,8 +2269,8 @@ static void handle_error_indication(union ibmvnic_crq 
*crq,
dev_err(dev, "Firmware reports %serror id %x, cause %d\n",
crq->error_indication.
flags & IBMVNIC_FATAL_ERROR ? "FATAL " : "",
-   crq->error_indication.error_id,
-   crq->error_indication.error_cause);
+   be32_to_cpu(crq->error_indication.error_id),
+   be16_to_cpu(crq->error_indication.error_cause));
 
error_buff = kmalloc(sizeof(*error_buff), GFP_ATOMIC);
if (!error_buff)
-- 
1.8.3.1

[PATCH net 5/5] ibmvnic: init completion struct before requesting long term mapped buffers

[Thomas Falcon]

Initialize this completion structure before requesting that
a buffer be long-term mapped . This fix resolves a bug where firmware
sends a response before the structure is initialized.

Signed-off-by: John Allen <jal...@linux.vnet.ibm.com>
Signed-off-by: Nathan Fontenot <nf...@linux.vnet.ibm.com>
Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index ec6c5fe..d1ffc61 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -189,9 +189,9 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
}
ltb->map_id = adapter->map_id;
adapter->map_id++;
+   init_completion(>fw_done);
send_request_map(adapter, ltb->addr,
 ltb->size, ltb->map_id);
-   init_completion(>fw_done);
wait_for_completion(>fw_done);
return 0;
 }
-- 
1.8.3.1

[PATCH] ibmvnic: Handle backing device failover and reinitialization

[Thomas Falcon]

An upcoming feature of IBM VNIC protocol is the ability to configure
redundant backing devices for a VNIC client. In case of a failure
on the current backing device, the driver will receive a signal
from the hypervisor indicating that a failover will occur. The driver
will then wait for a message from the backing device before 
establishing a new connection.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 34 --
 drivers/net/ethernet/ibm/ibmvnic.h |  2 ++
 2 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 88f3c85..b942108 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -203,7 +203,8 @@ static void free_long_term_buff(struct ibmvnic_adapter 
*adapter,
struct device *dev = >vdev->dev;
 
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
-   send_request_unmap(adapter, ltb->map_id);
+   if (!adapter->failover)
+   send_request_unmap(adapter, ltb->map_id);
 }
 
 static int alloc_rx_pool(struct ibmvnic_adapter *adapter,
@@ -522,7 +523,8 @@ static int ibmvnic_close(struct net_device *netdev)
for (i = 0; i < adapter->req_rx_queues; i++)
napi_disable(>napi[i]);
 
-   netif_tx_stop_all_queues(netdev);
+   if (!adapter->failover)
+   netif_tx_stop_all_queues(netdev);
 
if (adapter->bounce_buffer) {
if (!dma_mapping_error(dev, adapter->bounce_buffer_dma)) {
@@ -3280,6 +3282,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
rc = ibmvnic_send_crq_init(adapter);
if (rc)
dev_err(dev, "Error sending init rc=%ld\n", rc);
+   } else if (gen_crq->cmd == IBMVNIC_DEVICE_FAILOVER) {
+   dev_info(dev, "Backing device failover detected\n");
+   netif_carrier_off(netdev);
+   adapter->failover = true;
} else {
/* The adapter lost the connection */
dev_err(dev, "Virtual Adapter failed (rc=%d)\n",
@@ -3615,8 +3621,18 @@ static void handle_crq_init_rsp(struct work_struct *work)
struct device *dev = >vdev->dev;
struct net_device *netdev = adapter->netdev;
unsigned long timeout = msecs_to_jiffies(3);
+   bool restart = false;
int rc;
 
+   if (adapter->failover) {
+   release_sub_crqs(adapter);
+   if (netif_running(netdev)) {
+   netif_tx_disable(netdev);
+   ibmvnic_close(netdev);
+   restart = true;
+   }
+   }
+
send_version_xchg(adapter);
reinit_completion(>init_done);
if (!wait_for_completion_timeout(>init_done, timeout)) {
@@ -3645,6 +3661,17 @@ static void handle_crq_init_rsp(struct work_struct *work)
 
netdev->real_num_tx_queues = adapter->req_tx_queues;
 
+   if (adapter->failover) {
+   adapter->failover = false;
+   if (restart) {
+   rc = ibmvnic_open(netdev);
+   if (rc)
+   goto restart_failed;
+   }
+   netif_carrier_on(netdev);
+   return;
+   }
+
rc = register_netdev(netdev);
if (rc) {
dev_err(dev,
@@ -3655,6 +3682,8 @@ static void handle_crq_init_rsp(struct work_struct *work)
 
return;
 
+restart_failed:
+   dev_err(dev, "Failed to restart ibmvnic, rc=%d\n", rc);
 register_failed:
release_sub_crqs(adapter);
 task_failed:
@@ -3692,6 +3721,7 @@ static int ibmvnic_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
dev_set_drvdata(>dev, netdev);
adapter->vdev = dev;
adapter->netdev = netdev;
+   adapter->failover = false;
 
ether_addr_copy(adapter->mac_addr, mac_addr_p);
ether_addr_copy(netdev->dev_addr, adapter->mac_addr);
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h 
b/drivers/net/ethernet/ibm/ibmvnic.h
index e82898f..bfc84c7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -830,6 +830,7 @@ enum ibmvfc_crq_format {
IBMVNIC_CRQ_INIT = 0x01,
IBMVNIC_CRQ_INIT_COMPLETE= 0x02,
IBMVNIC_PARTITION_MIGRATED   = 0x06,
+   IBMVNIC_DEVICE_FAILOVER  = 0x08,
 };
 
 struct ibmvnic_crq_queue {
@@ -1047,4 +1048,5 @@ struct ibmvnic_adapter {
u8 map_id;
 
struct work_struct vnic_crq_init;
+   bool failover;
 };
-- 
1.8.3.1

Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type

[Thomas Falcon]

On 10/27/2016 10:26 AM, Eric Dumazet wrote:
> On Wed, 2016-10-26 at 11:09 +1100, Jon Maxwell wrote:
>> We recently encountered a bug where a few customers using ibmveth on the 
>> same LPAR hit an issue where a TCP session hung when large receive was
>> enabled. Closer analysis revealed that the session was stuck because the 
>> one side was advertising a zero window repeatedly.
>>
>> We narrowed this down to the fact the ibmveth driver did not set gso_size 
>> which is translated by TCP into the MSS later up the stack. The MSS is 
>> used to calculate the TCP window size and as that was abnormally large, 
>> it was calculating a zero window, even although the sockets receive buffer 
>> was completely empty. 
>>
>> We were able to reproduce this and worked with IBM to fix this. Thanks Tom 
>> and Marcelo for all your help and review on this.
>>
>> The patch fixes both our internal reproduction tests and our customers tests.
>>
>> Signed-off-by: Jon Maxwell 
>> ---
>>  drivers/net/ethernet/ibm/ibmveth.c | 20 
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
>> b/drivers/net/ethernet/ibm/ibmveth.c
>> index 29c05d0..c51717e 100644
>> --- a/drivers/net/ethernet/ibm/ibmveth.c
>> +++ b/drivers/net/ethernet/ibm/ibmveth.c
>> @@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int 
>> budget)
>>  int frames_processed = 0;
>>  unsigned long lpar_rc;
>>  struct iphdr *iph;
>> +bool large_packet = 0;
>> +u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
>>  
>>  restart_poll:
>>  while (frames_processed < budget) {
>> @@ -1236,10 +1238,28 @@ static int ibmveth_poll(struct napi_struct *napi, 
>> int budget)
>>  iph->check = 0;
>>  iph->check = 
>> ip_fast_csum((unsigned char *)iph, iph->ihl);
>>  adapter->rx_large_packets++;
>> +large_packet = 1;
>>  }
>>  }
>>  }
>>  
>> +if (skb->len > netdev->mtu) {
>> +iph = (struct iphdr *)skb->data;
>> +if (be16_to_cpu(skb->protocol) == ETH_P_IP &&
>> +iph->protocol == IPPROTO_TCP) {
>> +hdr_len += sizeof(struct iphdr);
>> +skb_shinfo(skb)->gso_type = 
>> SKB_GSO_TCPV4;
>> +skb_shinfo(skb)->gso_size = netdev->mtu 
>> - hdr_len;
>> +} else if (be16_to_cpu(skb->protocol) == 
>> ETH_P_IPV6 &&
>> +   iph->protocol == IPPROTO_TCP) {
>> +hdr_len += sizeof(struct ipv6hdr);
>> +skb_shinfo(skb)->gso_type = 
>> SKB_GSO_TCPV6;
>> +skb_shinfo(skb)->gso_size = netdev->mtu 
>> - hdr_len;
>> +}
>> +if (!large_packet)
>> +adapter->rx_large_packets++;
>> +}
>> +
>>  
> This might break forwarding and PMTU discovery.
>
> You force gso_size to device mtu, regardless of real MSS used by the TCP
> sender.
>
> Don't you have the MSS provided in RX descriptor, instead of guessing
> the value ?
>
>
>
The MSS is not always available unfortunately, so this is the best solution 
there is at the moment. 

Re: [bug report] Driver for IBM System i/p VNIC protocol

[Thomas Falcon]

On 11/16/2016 06:25 AM, Dan Carpenter wrote:
> Hello Thomas Falcon,
>
> The patch 032c5e82847a: "Driver for IBM System i/p VNIC protocol"
> from Dec 21, 2015, leads to the following static checker warning:
>
>   drivers/net/ethernet/ibm/ibmvnic.c:2957 error_level_write()
>   why cast 'kstrtoul()?'
>
> drivers/net/ethernet/ibm/ibmvnic.c
>   2946  static ssize_t error_level_write(struct file *file, const char __user 
> *user_buf,
>   2947   size_t len, loff_t *ppos)
>   2948  {
>   2949  struct ibmvnic_fw_comp_internal *ras_comp_int = 
> file->private_data;
>   2950  struct ibmvnic_adapter *adapter = ras_comp_int->adapter;
>   2951  int num = ras_comp_int->num;
>   2952  union ibmvnic_crq crq;
>   2953  unsigned long val;
>   2954  char buff[9]; /* decimal max int plus \n and \0 */
>   2955  
>   2956  copy_from_user(buff, user_buf, sizeof(buff));
>
> No error checking.
>
>   2957  val = kstrtoul(buff, 10, NULL);
>
> This is a wrong conversion from simple_strtoul().  The code has clearly
> never been tested.  There are four other buggy untested calls to
> kstrtoul() in this file.
>
>   2958  
>   2959  if (val > 9)
>   2960  val = 9;
>   2961  
>   2962  memset(, 0, sizeof(crq));
>   2963  crq.control_ras.first = IBMVNIC_CRQ_CMD;
>   2964  crq.control_ras.cmd = CONTROL_RAS;
>   2965  crq.control_ras.correlator = 
> adapter->ras_comps[num].correlator;
>   2966  crq.control_ras.op = IBMVNIC_ERROR_LEVEL;
>   2967  crq.control_ras.level = val;
>   2968  ibmvnic_send_crq(adapter, );
>   2969  
>   2970  return len;
>   2971  }
Thank you for your time and attention, Dan.  I had also noticed these errors, 
but did not immediately fix it after being told by our firmware team that 
support had been discontinued for these features.  I plan to remove/fix these 
soon.

Thanks again,
Tom


> regards,
> dan carpenter
>

Re: [PATCH net-next] ibmveth: v1 calculate correct gso_size and set gso_type

[Thomas Falcon]

On 10/25/2016 07:09 PM, Jon Maxwell wrote:
> We recently encountered a bug where a few customers using ibmveth on the 
> same LPAR hit an issue where a TCP session hung when large receive was
> enabled. Closer analysis revealed that the session was stuck because the 
> one side was advertising a zero window repeatedly.
>
> We narrowed this down to the fact the ibmveth driver did not set gso_size 
> which is translated by TCP into the MSS later up the stack. The MSS is 
> used to calculate the TCP window size and as that was abnormally large, 
> it was calculating a zero window, even although the sockets receive buffer 
> was completely empty. 
>
> We were able to reproduce this and worked with IBM to fix this. Thanks Tom 
> and Marcelo for all your help and review on this.
>
> The patch fixes both our internal reproduction tests and our customers tests.
>
> Signed-off-by: Jon Maxwell <jmaxwel...@gmail.com>
Thanks, Jon.

Acked-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>

> ---
>  drivers/net/ethernet/ibm/ibmveth.c | 20 
>  1 file changed, 20 insertions(+)
>
> diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
> b/drivers/net/ethernet/ibm/ibmveth.c
> index 29c05d0..c51717e 100644
> --- a/drivers/net/ethernet/ibm/ibmveth.c
> +++ b/drivers/net/ethernet/ibm/ibmveth.c
> @@ -1182,6 +1182,8 @@ static int ibmveth_poll(struct napi_struct *napi, int 
> budget)
>   int frames_processed = 0;
>   unsigned long lpar_rc;
>   struct iphdr *iph;
> + bool large_packet = 0;
> + u16 hdr_len = ETH_HLEN + sizeof(struct tcphdr);
>
>  restart_poll:
>   while (frames_processed < budget) {
> @@ -1236,10 +1238,28 @@ static int ibmveth_poll(struct napi_struct *napi, int 
> budget)
>   iph->check = 0;
>   iph->check = 
> ip_fast_csum((unsigned char *)iph, iph->ihl);
>   adapter->rx_large_packets++;
> + large_packet = 1;
>   }
>   }
>   }
>
> + if (skb->len > netdev->mtu) {
> + iph = (struct iphdr *)skb->data;
> + if (be16_to_cpu(skb->protocol) == ETH_P_IP &&
> + iph->protocol == IPPROTO_TCP) {
> + hdr_len += sizeof(struct iphdr);
> + skb_shinfo(skb)->gso_type = 
> SKB_GSO_TCPV4;
> + skb_shinfo(skb)->gso_size = netdev->mtu 
> - hdr_len;
> + } else if (be16_to_cpu(skb->protocol) == 
> ETH_P_IPV6 &&
> +iph->protocol == IPPROTO_TCP) {
> + hdr_len += sizeof(struct ipv6hdr);
> + skb_shinfo(skb)->gso_type = 
> SKB_GSO_TCPV6;
> + skb_shinfo(skb)->gso_size = netdev->mtu 
> - hdr_len;
> + }
> + if (!large_packet)
> + adapter->rx_large_packets++;
> + }
> +
>   napi_gro_receive(napi, skb);/* send it up */
>
>   netdev->stats.rx_packets++;

Re: [PATCH FEAT] ibmvnic: Feature implementation of Vital Product Data (VPD) for the ibmvnic driver

[Thomas Falcon]

On 11/01/2017 09:39 AM, Desnes Augusto Nunes do Rosario wrote:
> This patch implements and enables VDP support for the ibmvnic driver. 
> Moreover, it includes the implementation of suitable structs, signal 
> transmission/handling and fuctions which allows the retrival of firmware 
> information from the ibmvnic card.
>
> Co-Authored-By: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> ---

Hi, you should include a Signed-off-by tag in the commit message.  You should 
also include the branch the patch is meant for in the PATCH field.  In this 
case, it would be net-next. The commit message should also be wrapped (75 char 
per line), and 'fuctions' is missing an n.

>  drivers/net/ethernet/ibm/ibmvnic.c | 140 
> -
>  drivers/net/ethernet/ibm/ibmvnic.h |  27 ++-
>  2 files changed, 164 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index d0cff28..120f3c0 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -107,6 +107,9 @@ static union sub_crq *ibmvnic_next_scrq(struct 
> ibmvnic_adapter *,
>   struct ibmvnic_sub_crq_queue *);
>  static int ibmvnic_poll(struct napi_struct *napi, int data);
>  static void send_map_query(struct ibmvnic_adapter *adapter);
> +static int ibmvnic_get_vpd(struct ibmvnic_adapter *);
> +static void handle_vpd_size_rsp(union ibmvnic_crq *, struct ibmvnic_adapter 
> *);
> +static void handle_vpd_rsp(union ibmvnic_crq *,struct ibmvnic_adapter *);

There should be a space after the comma.

>  static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, 
> u8);
>  static void send_request_unmap(struct ibmvnic_adapter *, u8);
>  static void send_login(struct ibmvnic_adapter *adapter);
> @@ -573,6 +576,15 @@ static int reset_tx_pools(struct ibmvnic_adapter 
> *adapter)
>   return 0;
>  }
>
> +static void release_vpd_data(struct ibmvnic_adapter *adapter)
> +{
> + if(!adapter->vpd)

There should be a space between the if here. There are also some style checks 
that were picked up by checkpatch.pl. It's a good idea to run your patch 
through that before submission.

Thanks,
Tom

> + return;
> +
> + kfree(adapter->vpd->buff);
> + kfree(adapter->vpd);
> +}
> +
>  static void release_tx_pools(struct ibmvnic_adapter *adapter)
>  {
>   struct ibmvnic_tx_pool *tx_pool;
> @@ -753,6 +765,8 @@ static void release_resources(struct ibmvnic_adapter 
> *adapter)
>  {
>   int i;
>
> + release_vpd_data(adapter);
> +
>   release_tx_pools(adapter);
>   release_rx_pools(adapter);
>
> @@ -850,6 +864,10 @@ static int init_resources(struct ibmvnic_adapter 
> *adapter)
>   if (rc)
>   return rc;
>
> + adapter->vpd = kzalloc(sizeof(struct ibmvnic_vpd), GFP_KERNEL);
> + if (!adapter->vpd)
> + return -ENOMEM;
> +
>   adapter->map_id = 1;
>   adapter->napi = kcalloc(adapter->req_rx_queues,
>   sizeof(struct napi_struct), GFP_KERNEL);
> @@ -950,6 +968,10 @@ static int ibmvnic_open(struct net_device *netdev)
>
>   rc = __ibmvnic_open(netdev);
>   netif_carrier_on(netdev);
> +
> + /* Vital Product Data (VPD) */
> + ibmvnic_get_vpd(adapter);
> +
>   mutex_unlock(>reset_lock);
>
>   return rc;
> @@ -1878,11 +1900,15 @@ static int ibmvnic_get_link_ksettings(struct 
> net_device *netdev,
>   return 0;
>  }
>
> -static void ibmvnic_get_drvinfo(struct net_device *dev,
> +static void ibmvnic_get_drvinfo(struct net_device *netdev,
>   struct ethtool_drvinfo *info)
>  {
> + struct ibmvnic_adapter *adapter = netdev_priv(netdev);
> +
>   strlcpy(info->driver, ibmvnic_driver_name, sizeof(info->driver));
>   strlcpy(info->version, IBMVNIC_DRIVER_VERSION, sizeof(info->version));
> + strlcpy(info->fw_version, adapter->fw_version,
> + sizeof(info->fw_version));
>  }
>
>  static u32 ibmvnic_get_msglevel(struct net_device *netdev)
> @@ -2923,6 +2947,110 @@ static void send_login(struct ibmvnic_adapter 
> *adapter)
>   return;
>  }
>
> +static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
> +{
> + struct device *dev = >vdev->dev;
> + union ibmvnic_crq crq;
> + dma_addr_t dma_addr;
> + int len;
> +
> + if (adapter->vpd->buff)
> + len = adapter->vpd->len;
> +
> + reinit_completion(>fw_done);
> + crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
> + crq.get_vpd_size.cmd = 

Re: [RFC v5 3/6] migration/dlpar: Add device readd queuing function

[Thomas Falcon]

On 05/21/2018 12:52 PM, Michael Bringmann wrote:
> migration/dlpar: This patch adds function dlpar_readd_action()
> which will queue a worker function to 'readd' a device in the
> system.  Such devices must be identified by a 'resource' type
> and a drc_index to be readded.

The function in the commit message and the patch have different names. The 
patch seems to queue a generic action instead of a readd.  The commit message 
needs to be updated to describe this new function.

Tom

>
> Signed-off-by: Michael Bringmann 
> ---
>  arch/powerpc/platforms/pseries/dlpar.c   |   14 ++
>  arch/powerpc/platforms/pseries/pseries.h |1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c 
> b/arch/powerpc/platforms/pseries/dlpar.c
> index a0b20c0..a14684e 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -407,6 +407,20 @@ void queue_hotplug_event(struct pseries_hp_errorlog 
> *hp_errlog,
>   }
>  }
>
> +int dlpar_queue_action(int resource, int action, u32 drc_index)
> +{
> + struct pseries_hp_errorlog hp_elog;
> +
> + hp_elog.resource = resource;
> + hp_elog.action = action;
> + hp_elog.id_type = PSERIES_HP_ELOG_ID_DRC_INDEX;
> + hp_elog._drc_u.drc_index = drc_index;
> +
> + queue_hotplug_event(_elog, NULL, NULL);
> +
> + return 0;
> +}
> +
>  static int dlpar_parse_resource(char **cmd, struct pseries_hp_errorlog 
> *hp_elog)
>  {
>   char *arg;
> diff --git a/arch/powerpc/platforms/pseries/pseries.h 
> b/arch/powerpc/platforms/pseries/pseries.h
> index 60db2ee..cb2beb1 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -61,6 +61,7 @@ extern struct device_node *dlpar_configure_connector(__be32,
>
>  void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
>struct completion *hotplug_done, int *rc);
> +extern int dlpar_queue_action(int resource, int action, u32 drc_index);
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
>  #else

Re: [RFC v5 6/6] migration/memory: Update memory for assoc changes

[Thomas Falcon]

On 05/21/2018 12:52 PM, Michael Bringmann wrote:
> migration/memory: This patch adds more recognition for changes to
> the associativity of memory blocks described by the device-tree
> properties and updates local and general kernel data structures to
> reflect those changes.  These differences may include:
>
> * Evaluating 'ibm,dynamic-memory' properties when processing the
>   topology of LPARS in Post Migration events.  Previous efforts
>   only recognized whether a memory block's assignment had changed
>   in the property.  Changes here include checking the aa_index
>   values for each drc_index of the old/new LMBs and to 'readd'
>   any block for which the setting has changed.
>
> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
>   property may change.  In the event that a row of the array differs,
>   locate all assigned memory blocks with that 'aa_index' and 're-add'
>   them to the system memory block data structures.  In the process of
>   the 're-add', the system routines will update the corresponding entry
>   for the memory in the LMB structures and any other relevant kernel
>   data structures.
>
> * Extend the previous work for the 'ibm,associativity-lookup-array'
>   and 'ibm,dynamic-memory' properties to support the property
>   'ibm,dynamic-memory-v2' by means of the DRMEM LMB interpretation
>   code.
>
> Signed-off-by: Michael Bringmann 
> ---
> Changes in RFC:
>   -- Simplify code to update memory nodes during mobility checks.
>   -- Reuse code from DRMEM changes to scan for LMBs when updating
>  aa_index
>   -- Combine common code for properties 'ibm,dynamic-memory' and
>  'ibm,dynamic-memory-v2' after integrating DRMEM features.
>   -- Rearrange patches to co-locate memory property-related changes.
>   -- Use new paired list iterator for the drmem info arrays.
>   -- Use direct calls to add/remove memory from the update drconf
>  function as those operations are only intended for user DLPAR
>  ops, and should not occur during Migration reconfig notifier
>  changes.
>   -- Correct processing bug in processing of ibm,associativity-lookup-arrays
>   -- Rebase to 4.17-rc5 kernel
>   -- Apply minor code cleanups
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  153 
> ++-
>  1 file changed, 121 insertions(+), 32 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c 
> b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index c1578f5..ac329aa 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -994,13 +994,11 @@ static int pseries_add_mem_node(struct device_node *np)
>   return (ret < 0) ? -EINVAL : 0;
>  }
>
> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
>  {
> - struct of_drconf_cell_v1 *new_drmem, *old_drmem;
> + struct drmem_lmb *old_lmb, *new_lmb;
>   unsigned long memblock_size;
> - u32 entries;
> - __be32 *p;
> - int i, rc = -EINVAL;
> + int rc = 0;
>
>   if (rtas_hp_event)
>   return 0;
> @@ -1009,42 +1007,124 @@ static int pseries_update_drconf_memory(struct 
> of_reconfig_data *pr)
>   if (!memblock_size)
>   return -EINVAL;
>
> - p = (__be32 *) pr->old_prop->value;
> - if (!p)
> - return -EINVAL;
> + /* Arrays should have the same size and DRC indexes */
> + for_each_pair_drmem_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
>
> - /* The first int of the property is the number of lmb's described
> -  * by the property. This is followed by an array of of_drconf_cell
> -  * entries. Get the number of entries and skip to the array of
> -  * of_drconf_cell's.
> -  */
> - entries = be32_to_cpu(*p++);
> - old_drmem = (struct of_drconf_cell_v1 *)p;
> -
> - p = (__be32 *)pr->prop->value;
> - p++;
> - new_drmem = (struct of_drconf_cell_v1 *)p;
> + if (new_lmb->drc_index != old_lmb->drc_index)
> + continue;
>
> - for (i = 0; i < entries; i++) {
> - if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
> - (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) 
> {
> + if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
> + (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
>   rc = pseries_remove_memblock(
> - be64_to_cpu(old_drmem[i].base_addr),
> -  memblock_size);
> + old_lmb->base_addr, memblock_size);
>   break;
> - } else if ((!(be32_to_cpu(old_drmem[i].flags) &
> - DRCONF_MEM_ASSIGNED)) &&
> - (be32_to_cpu(new_drmem[i].flags) &
> - 

[PATCH net-next 0/8] ibmvnic: Failover hardening

[Thomas Falcon]

Introduce additional transport event hardening to handle
events during device reset. In the driver's current state,
if a transport event is received during device reset, it can
cause the device to become unresponsive as invalid operations
are processed as the backing device context changes. After
a transport event, the device expects a request to begin the
initialization process. If the driver is still processing
a previously queued device reset in this state, it is likely
to fail as firmware will reject any commands other than the
one to initialize the client driver's Command-Response Queue.

Instead of failing and becoming dormant, the driver will make
one more attempt to recover and continue operation. This is
achieved by setting a state flag, which if true will direct
the driver to clean up all allocated resources and perform
a hard reset in an attempt to bring the driver back to an
operational state.

Thomas Falcon (8):
  ibmvnic: Mark NAPI flag as disabled when released
  ibmvnic: Introduce active CRQ state
  ibmvnic: Check CRQ command return codes
  ibmvnic: Return error code if init interrupted by transport event
  ibmvnic: Handle error case when setting link state
  ibmvnic: Create separate initialization routine for resets
  ibmvnic: Set resetting state at earliest possible point
  ibmvnic: Introduce hard reset recovery

 drivers/net/ethernet/ibm/ibmvnic.c | 223 +
 drivers/net/ethernet/ibm/ibmvnic.h |   2 +
 2 files changed, 202 insertions(+), 23 deletions(-)

-- 
2.7.5

[PATCH net-next 1/8] ibmvnic: Mark NAPI flag as disabled when released

[Thomas Falcon]

Set adapter NAPI state as disabled if they are removed. This will allow
them to be enabled again if reallocated in case of a hard reset.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 4bb4646..eabc1e4 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -789,6 +789,7 @@ static void release_napi(struct ibmvnic_adapter *adapter)
kfree(adapter->napi);
adapter->napi = NULL;
adapter->num_active_rx_napi = 0;
+   adapter->napi_enabled = false;
 }
 
 static int ibmvnic_login(struct net_device *netdev)
-- 
2.7.5

[PATCH net-next 3/8] ibmvnic: Check CRQ command return codes

[Thomas Falcon]

Check whether CRQ command is successful before awaiting a response
from the management partition. If the command was not successful, the
driver may hang waiting for a response that will never come.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 51 +++---
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index e6a081c..7083519 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -109,8 +109,8 @@ static union sub_crq *ibmvnic_next_scrq(struct 
ibmvnic_adapter *,
struct ibmvnic_sub_crq_queue *);
 static int ibmvnic_poll(struct napi_struct *napi, int data);
 static void send_map_query(struct ibmvnic_adapter *adapter);
-static void send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
-static void send_request_unmap(struct ibmvnic_adapter *, u8);
+static int send_request_map(struct ibmvnic_adapter *, dma_addr_t, __be32, u8);
+static int send_request_unmap(struct ibmvnic_adapter *, u8);
 static int send_login(struct ibmvnic_adapter *adapter);
 static void send_cap_queries(struct ibmvnic_adapter *adapter);
 static int init_sub_crqs(struct ibmvnic_adapter *);
@@ -172,6 +172,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
struct ibmvnic_long_term_buff *ltb, int size)
 {
struct device *dev = >vdev->dev;
+   int rc;
 
ltb->size = size;
ltb->buff = dma_alloc_coherent(dev, ltb->size, >addr,
@@ -185,8 +186,12 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
adapter->map_id++;
 
init_completion(>fw_done);
-   send_request_map(adapter, ltb->addr,
-ltb->size, ltb->map_id);
+   rc = send_request_map(adapter, ltb->addr,
+ ltb->size, ltb->map_id);
+   if (rc) {
+   dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+   return rc;
+   }
wait_for_completion(>fw_done);
 
if (adapter->fw_done_rc) {
@@ -215,10 +220,14 @@ static void free_long_term_buff(struct ibmvnic_adapter 
*adapter,
 static int reset_long_term_buff(struct ibmvnic_adapter *adapter,
struct ibmvnic_long_term_buff *ltb)
 {
+   int rc;
+
memset(ltb->buff, 0, ltb->size);
 
init_completion(>fw_done);
-   send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+   rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
+   if (rc)
+   return rc;
wait_for_completion(>fw_done);
 
if (adapter->fw_done_rc) {
@@ -952,6 +961,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
struct device *dev = >vdev->dev;
union ibmvnic_crq crq;
int len = 0;
+   int rc;
 
if (adapter->vpd->buff)
len = adapter->vpd->len;
@@ -959,7 +969,9 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter *adapter)
init_completion(>fw_done);
crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
crq.get_vpd_size.cmd = GET_VPD_SIZE;
-   ibmvnic_send_crq(adapter, );
+   rc = ibmvnic_send_crq(adapter, );
+   if (rc)
+   return rc;
wait_for_completion(>fw_done);
 
if (!adapter->vpd->len)
@@ -992,7 +1004,12 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter 
*adapter)
crq.get_vpd.cmd = GET_VPD;
crq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);
crq.get_vpd.len = cpu_to_be32((u32)adapter->vpd->len);
-   ibmvnic_send_crq(adapter, );
+   rc = ibmvnic_send_crq(adapter, );
+   if (rc) {
+   kfree(adapter->vpd->buff);
+   adapter->vpd->buff = NULL;
+   return rc;
+   }
wait_for_completion(>fw_done);
 
return 0;
@@ -1691,6 +1708,7 @@ static int __ibmvnic_set_mac(struct net_device *netdev, 
struct sockaddr *p)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
struct sockaddr *addr = p;
union ibmvnic_crq crq;
+   int rc;
 
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
@@ -1701,7 +1719,9 @@ static int __ibmvnic_set_mac(struct net_device *netdev, 
struct sockaddr *p)
ether_addr_copy(_mac_addr.mac_addr[0], addr->sa_data);
 
init_completion(>fw_done);
-   ibmvnic_send_crq(adapter, );
+   rc = ibmvnic_send_crq(adapter, );
+   if (rc)
+   return rc;
wait_for_completion(>fw_done);
/* netdev->dev_addr is changed in handle_change_mac_rsp function */
return adapter->fw_done_rc ? -EIO : 0;
@@ -2365,6 +2385,7 @@

[PATCH net-next 2/8] ibmvnic: Introduce active CRQ state

[Thomas Falcon]

Introduce an "active" state for a IBM vNIC Command-Response Queue. A CRQ
is considered active once it has initialized or linked with its partner by
sending an initialization request and getting a successful response back
from the management partition.  Until this has happened, do not allow CRQ
commands to be sent other than the initialization request.

This change will avoid a protocol error in case of a device transport
event occurring during a initialization. When the driver receives a
transport event notification indicating that the backing hardware
has changed and needs reinitialization, any further commands other
than the initialization handshake with the VIOS management partition
will result in an invalid state error. Instead of sending a command
that will be returned with an error, print a warning and return an
error that will be handled by the caller.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 10 ++
 drivers/net/ethernet/ibm/ibmvnic.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index eabc1e4..e6a081c 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -3147,6 +3147,12 @@ static int ibmvnic_send_crq(struct ibmvnic_adapter 
*adapter,
   (unsigned long int)cpu_to_be64(u64_crq[0]),
   (unsigned long int)cpu_to_be64(u64_crq[1]));
 
+   if (!adapter->crq.active &&
+   crq->generic.first != IBMVNIC_CRQ_INIT_CMD) {
+   dev_warn(dev, "Invalid request detected while CRQ is inactive, 
possible device state change during reset\n");
+   return -EINVAL;
+   }
+
/* Make sure the hypervisor sees the complete request */
mb();
 
@@ -4225,6 +4231,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
break;
case IBMVNIC_CRQ_INIT_COMPLETE:
dev_info(dev, "Partner initialization complete\n");
+   adapter->crq.active = true;
send_version_xchg(adapter);
break;
default:
@@ -4233,6 +4240,7 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
return;
case IBMVNIC_CRQ_XPORT_EVENT:
netif_carrier_off(netdev);
+   adapter->crq.active = false;
if (gen_crq->cmd == IBMVNIC_PARTITION_MIGRATED) {
dev_info(dev, "Migrated, re-enabling adapter\n");
ibmvnic_reset(adapter, VNIC_RESET_MOBILITY);
@@ -4420,6 +4428,7 @@ static int ibmvnic_reset_crq(struct ibmvnic_adapter 
*adapter)
/* Clean out the queue */
memset(crq->msgs, 0, PAGE_SIZE);
crq->cur = 0;
+   crq->active = false;
 
/* And re-open it again */
rc = plpar_hcall_norets(H_REG_CRQ, vdev->unit_address,
@@ -4454,6 +4463,7 @@ static void release_crq_queue(struct ibmvnic_adapter 
*adapter)
 DMA_BIDIRECTIONAL);
free_page((unsigned long)crq->msgs);
crq->msgs = NULL;
+   crq->active = false;
 }
 
 static int init_crq_queue(struct ibmvnic_adapter *adapter)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.h 
b/drivers/net/ethernet/ibm/ibmvnic.h
index 22391e8..edfc312 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.h
+++ b/drivers/net/ethernet/ibm/ibmvnic.h
@@ -865,6 +865,7 @@ struct ibmvnic_crq_queue {
int size, cur;
dma_addr_t msg_token;
spinlock_t lock;
+   bool active;
 };
 
 union sub_crq {
-- 
2.7.5

[PATCH net-next 4/8] ibmvnic: Return error code if init interrupted by transport event

[Thomas Falcon]

If device init is interrupted by a failover, set the init return
code so that it can be checked and handled appropriately by the
init routine.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 7083519..f1f744e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -4249,7 +4249,10 @@ static void ibmvnic_handle_crq(union ibmvnic_crq *crq,
dev_info(dev, "Partner initialized\n");
adapter->from_passive_init = true;
adapter->failover_pending = false;
-   complete(>init_done);
+   if (!completion_done(>init_done)) {
+   complete(>init_done);
+   adapter->init_done_rc = -EIO;
+   }
ibmvnic_reset(adapter, VNIC_RESET_FAILOVER);
break;
case IBMVNIC_CRQ_INIT_COMPLETE:
-- 
2.7.5

[PATCH net-next 5/8] ibmvnic: Handle error case when setting link state

[Thomas Falcon]

If setting the link state is not successful, print a warning
with the resulting return code and return it to be handled
by the caller.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index f1f744e..b1bbd5b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -929,6 +929,10 @@ static int set_link_state(struct ibmvnic_adapter *adapter, 
u8 link_state)
/* Partuial success, delay and re-send */
mdelay(1000);
resend = true;
+   } else if (adapter->init_done_rc) {
+   netdev_warn(netdev, "Unable to set link state, rc=%d\n",
+   adapter->init_done_rc);
+   return adapter->init_done_rc;
}
} while (resend);
 
-- 
2.7.5

[PATCH net-next 6/8] ibmvnic: Create separate initialization routine for resets

[Thomas Falcon]

Instead of having one initialization routine for all cases, create
a separate, simpler function for standard initialization, such as during
device probe. Use the original initialization function to handle
device reset scenarios. The goal of this patch is to avoid having
a single, cluttered init function to handle all possible
scenarios.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 48 --
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index b1bbd5b..f26e1f8 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -116,6 +116,7 @@ static void send_cap_queries(struct ibmvnic_adapter 
*adapter);
 static int init_sub_crqs(struct ibmvnic_adapter *);
 static int init_sub_crq_irqs(struct ibmvnic_adapter *adapter);
 static int ibmvnic_init(struct ibmvnic_adapter *);
+static int ibmvnic_reset_init(struct ibmvnic_adapter *);
 static void release_crq_queue(struct ibmvnic_adapter *);
 static int __ibmvnic_set_mac(struct net_device *netdev, struct sockaddr *p);
 static int init_crq_queue(struct ibmvnic_adapter *adapter);
@@ -1807,7 +1808,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return rc;
}
 
-   rc = ibmvnic_init(adapter);
+   rc = ibmvnic_reset_init(adapter);
if (rc)
return IBMVNIC_INIT_FAILED;
 
@@ -4571,7 +4572,7 @@ static int init_crq_queue(struct ibmvnic_adapter *adapter)
return retrc;
 }
 
-static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+static int ibmvnic_reset_init(struct ibmvnic_adapter *adapter)
 {
struct device *dev = >vdev->dev;
unsigned long timeout = msecs_to_jiffies(3);
@@ -4630,6 +4631,49 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
return rc;
 }
 
+static int ibmvnic_init(struct ibmvnic_adapter *adapter)
+{
+   struct device *dev = >vdev->dev;
+   unsigned long timeout = msecs_to_jiffies(3);
+   int rc;
+
+   adapter->from_passive_init = false;
+
+   init_completion(>init_done);
+   adapter->init_done_rc = 0;
+   ibmvnic_send_crq_init(adapter);
+   if (!wait_for_completion_timeout(>init_done, timeout)) {
+   dev_err(dev, "Initialization sequence timed out\n");
+   return -1;
+   }
+
+   if (adapter->init_done_rc) {
+   release_crq_queue(adapter);
+   return adapter->init_done_rc;
+   }
+
+   if (adapter->from_passive_init) {
+   adapter->state = VNIC_OPEN;
+   adapter->from_passive_init = false;
+   return -1;
+   }
+
+   rc = init_sub_crqs(adapter);
+   if (rc) {
+   dev_err(dev, "Initialization of sub crqs failed\n");
+   release_crq_queue(adapter);
+   return rc;
+   }
+
+   rc = init_sub_crq_irqs(adapter);
+   if (rc) {
+   dev_err(dev, "Failed to initialize sub crq irqs\n");
+   release_crq_queue(adapter);
+   }
+
+   return rc;
+}
+
 static struct device_attribute dev_attr_failover;
 
 static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
-- 
2.7.5

[PATCH net-next 8/8] ibmvnic: Introduce hard reset recovery

[Thomas Falcon]

Introduce a recovery hard reset to handle reset failure as a result of
change of device context following a transport event, such as a
backing device failover or partition migration. These operations reset
the device context to its initial state. If this occurs during a reset,
any initialization commands are likely to fail with an invalid state
error as backing device firmware requests reinitialization.

When this happens, make one more attempt by performing a hard reset,
which frees any resources currently allocated and performs device
initialization. If a transport event occurs during a device reset, a
flag is set which will trigger a new hard reset following the
completionof the current reset event.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 101 +++--
 drivers/net/ethernet/ibm/ibmvnic.h |   1 +
 2 files changed, 98 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index ee51deb..09f8e6b 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1878,6 +1878,85 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return 0;
 }
 
+static int do_hard_reset(struct ibmvnic_adapter *adapter,
+struct ibmvnic_rwi *rwi, u32 reset_state)
+{
+   struct net_device *netdev = adapter->netdev;
+   int rc;
+
+   netdev_dbg(adapter->netdev, "Hard resetting driver (%d)\n",
+  rwi->reset_reason);
+
+   netif_carrier_off(netdev);
+   adapter->reset_reason = rwi->reset_reason;
+
+   ibmvnic_cleanup(netdev);
+   release_resources(adapter);
+   release_sub_crqs(adapter, 0);
+   release_crq_queue(adapter);
+
+   /* remove the closed state so when we call open it appears
+* we are coming from the probed state.
+*/
+   adapter->state = VNIC_PROBED;
+
+   rc = init_crq_queue(adapter);
+   if (rc) {
+   netdev_err(adapter->netdev,
+  "Couldn't initialize crq. rc=%d\n", rc);
+   return rc;
+   }
+
+   rc = ibmvnic_init(adapter);
+   if (rc)
+   return rc;
+
+   /* If the adapter was in PROBE state prior to the reset,
+* exit here.
+*/
+   if (reset_state == VNIC_PROBED)
+   return 0;
+
+   rc = ibmvnic_login(netdev);
+   if (rc) {
+   adapter->state = VNIC_PROBED;
+   return 0;
+   }
+   /* netif_set_real_num_xx_queues needs to take rtnl lock here
+* unless wait_for_reset is set, in which case the rtnl lock
+* has already been taken before initializing the reset
+*/
+   if (!adapter->wait_for_reset) {
+   rtnl_lock();
+   rc = init_resources(adapter);
+   rtnl_unlock();
+   } else {
+   rc = init_resources(adapter);
+   }
+   if (rc)
+   return rc;
+
+   ibmvnic_disable_irqs(adapter);
+   adapter->state = VNIC_CLOSED;
+
+   if (reset_state == VNIC_CLOSED)
+   return 0;
+
+   rc = __ibmvnic_open(netdev);
+   if (rc) {
+   if (list_empty(>rwi_list))
+   adapter->state = VNIC_CLOSED;
+   else
+   adapter->state = reset_state;
+
+   return 0;
+   }
+
+   netif_carrier_on(netdev);
+
+   return 0;
+}
+
 static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter)
 {
struct ibmvnic_rwi *rwi;
@@ -1923,9 +2002,15 @@ static void __ibmvnic_reset(struct work_struct *work)
 
rwi = get_next_rwi(adapter);
while (rwi) {
-   rc = do_reset(adapter, rwi, reset_state);
+   if (adapter->force_reset_recovery) {
+   adapter->force_reset_recovery = false;
+   rc = do_hard_reset(adapter, rwi, reset_state);
+   } else {
+   rc = do_reset(adapter, rwi, reset_state);
+   }
kfree(rwi);
-   if (rc && rc != IBMVNIC_INIT_FAILED)
+   if (rc && rc != IBMVNIC_INIT_FAILED &&
+   !adapter->force_reset_recovery)
break;
 
rwi = get_next_rwi(adapter);
@@ -1951,9 +2036,9 @@ static void __ibmvnic_reset(struct work_struct *work)
 static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 enum ibmvnic_reset_reason reason)
 {
+   struct list_head *entry, *tmp_entry;
struct ibmvnic_rwi *rwi, *tmp;
struct net_device *netdev = adapter->netdev;
-   struct list_head *entry;
int ret;
 
if (adapter->state == VNIC_REMOVING ||
@@ -1989,7 +2074,13 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,

[PATCH net-next 7/8] ibmvnic: Set resetting state at earliest possible point

[Thomas Falcon]

Set device resetting state at the earliest possible point: as soon as a
reset is successfully scheduled. The reset state is toggled off when
all resets have been processed to completion.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index f26e1f8..ee51deb 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1919,7 +1919,6 @@ static void __ibmvnic_reset(struct work_struct *work)
netdev = adapter->netdev;
 
mutex_lock(>reset_lock);
-   adapter->resetting = true;
reset_state = adapter->state;
 
rwi = get_next_rwi(adapter);
@@ -1994,7 +1993,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
rwi->reset_reason = reason;
list_add_tail(>list, >rwi_list);
mutex_unlock(>rwi_lock);
-
+   adapter->resetting = true;
netdev_dbg(adapter->netdev, "Scheduling reset (reason %d)\n", reason);
schedule_work(>ibmvnic_reset);
 
-- 
2.7.5

[PATCH net] ibmvnic: Clean actual number of RX or TX pools

[Thomas Falcon]

Avoid using value stored in the login response buffer when
cleaning TX and RX buffer pools since these could be inconsistent
depending on the device state. Instead use the field in the driver's
private data that tracks the number of active pools.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 2df01ad..6e8d6a6 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1128,7 +1128,7 @@ static void clean_rx_pools(struct ibmvnic_adapter 
*adapter)
if (!adapter->rx_pool)
return;
 
-   rx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_rxadd_subcrqs);
+   rx_scrqs = adapter->num_active_rx_pools;
rx_entries = adapter->req_rx_add_entries_per_subcrq;
 
/* Free any remaining skbs in the rx buffer pools */
@@ -1177,7 +1177,7 @@ static void clean_tx_pools(struct ibmvnic_adapter 
*adapter)
if (!adapter->tx_pool || !adapter->tso_pool)
return;
 
-   tx_scrqs = be32_to_cpu(adapter->login_rsp_buf->num_txsubm_subcrqs);
+   tx_scrqs = adapter->num_active_tx_pools;
 
/* Free any remaining skbs in the tx buffer pools */
for (i = 0; i < tx_scrqs; i++) {
-- 
1.8.3.1

Re: [PATCH] ibmvnic: Clear pending interrupt after device reset

[Thomas Falcon]

On 04/15/2018 06:27 PM, Thomas Falcon wrote:
> Due to a firmware bug, the hypervisor can send an interrupt to a
> transmit or receive queue just prior to a partition migration, not
> allowing the device enough time to handle it and send an EOI. When
> the partition migrates, the interrupt is lost but an "EOI-pending"
> flag for the interrupt line is still set in firmware. No further
> interrupts will be sent until that flag is cleared, effectively
> freezing that queue. To workaround this, the driver will disable the
> hardware interrupt and send an H_EOI signal prior to re-enabling it.
> This will flush the pending EOI and allow the driver to continue
> operation.

Excuse me, I misspelled the linuxppc-dev email address.

Tom

> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> ---
>  drivers/net/ethernet/ibm/ibmvnic.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index f84a920..ef7995fc 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1034,16 +1034,14 @@ static int __ibmvnic_open(struct net_device *netdev)
>   netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
>   if (prev_state == VNIC_CLOSED)
>   enable_irq(adapter->rx_scrq[i]->irq);
> - else
> - enable_scrq_irq(adapter, adapter->rx_scrq[i]);
> + enable_scrq_irq(adapter, adapter->rx_scrq[i]);
>   }
>
>   for (i = 0; i < adapter->req_tx_queues; i++) {
>   netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
>   if (prev_state == VNIC_CLOSED)
>   enable_irq(adapter->tx_scrq[i]->irq);
> - else
> - enable_scrq_irq(adapter, adapter->tx_scrq[i]);
> + enable_scrq_irq(adapter, adapter->tx_scrq[i]);
>   }
>
>   rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
> @@ -1184,6 +1182,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter 
> *adapter)
>   if (adapter->tx_scrq[i]->irq) {
>   netdev_dbg(netdev,
>  "Disabling tx_scrq[%d] irq\n", i);
> + disable_scrq_irq(adapter, adapter->tx_scrq[i]);
>   disable_irq(adapter->tx_scrq[i]->irq);
>   }
>   }
> @@ -1193,6 +1192,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter 
> *adapter)
>   if (adapter->rx_scrq[i]->irq) {
>   netdev_dbg(netdev,
>  "Disabling rx_scrq[%d] irq\n", i);
> + disable_scrq_irq(adapter, adapter->rx_scrq[i]);
>   disable_irq(adapter->rx_scrq[i]->irq);
>   }
>   }
> @@ -2601,12 +2601,19 @@ static int enable_scrq_irq(struct ibmvnic_adapter 
> *adapter,
>  {
>   struct device *dev = >vdev->dev;
>   unsigned long rc;
> + u64 val;
>
>   if (scrq->hw_irq > 0x1ULL) {
>   dev_err(dev, "bad hw_irq = %lx\n", scrq->hw_irq);
>   return 1;
>   }
>
> + val = (0xff00) | scrq->hw_irq;
> + rc = plpar_hcall_norets(H_EOI, val);
> + if (rc)
> + dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n",
> + val, rc);
> +
>   rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
>   H_ENABLE_VIO_INTERRUPT, scrq->hw_irq, 0, 0);
>   if (rc)

Re: [PATCH] ibmvnic: Clear pending interrupt after device reset

[Thomas Falcon]

On 04/15/2018 07:55 PM, David Miller wrote:
> From: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Date: Sun, 15 Apr 2018 18:53:36 -0500
>
>> Due to a firmware bug, the hypervisor can send an interrupt to a
>> transmit or receive queue just prior to a partition migration, not
>> allowing the device enough time to handle it and send an EOI. When
>> the partition migrates, the interrupt is lost but an "EOI-pending"
>> flag for the interrupt line is still set in firmware. No further
>> interrupts will be sent until that flag is cleared, effectively
>> freezing that queue. To workaround this, the driver will disable the
>> hardware interrupt and send an H_EOI signal prior to re-enabling it.
>> This will flush the pending EOI and allow the driver to continue
>> operation.
>>
>> Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
> Hey Thomas, I see two copies of this patch posted.  Any special
> reason for that?
>
> Thanks.
>
Sorry, I misspelled one of the email addresses and mistakenly resent it.  
Please ignore this one.

[PATCH] ibmvnic: Clear pending interrupt after device reset

[Thomas Falcon]

Due to a firmware bug, the hypervisor can send an interrupt to a
transmit or receive queue just prior to a partition migration, not
allowing the device enough time to handle it and send an EOI. When
the partition migrates, the interrupt is lost but an "EOI-pending"
flag for the interrupt line is still set in firmware. No further
interrupts will be sent until that flag is cleared, effectively
freezing that queue. To workaround this, the driver will disable the
hardware interrupt and send an H_EOI signal prior to re-enabling it.
This will flush the pending EOI and allow the driver to continue
operation.

Signed-off-by: Thomas Falcon <tlfal...@linux.vnet.ibm.com>
---
 drivers/net/ethernet/ibm/ibmvnic.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index f84a920..ef7995fc 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1034,16 +1034,14 @@ static int __ibmvnic_open(struct net_device *netdev)
netdev_dbg(netdev, "Enabling rx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->rx_scrq[i]->irq);
-   else
-   enable_scrq_irq(adapter, adapter->rx_scrq[i]);
+   enable_scrq_irq(adapter, adapter->rx_scrq[i]);
}
 
for (i = 0; i < adapter->req_tx_queues; i++) {
netdev_dbg(netdev, "Enabling tx_scrq[%d] irq\n", i);
if (prev_state == VNIC_CLOSED)
enable_irq(adapter->tx_scrq[i]->irq);
-   else
-   enable_scrq_irq(adapter, adapter->tx_scrq[i]);
+   enable_scrq_irq(adapter, adapter->tx_scrq[i]);
}
 
rc = set_link_state(adapter, IBMVNIC_LOGICAL_LNK_UP);
@@ -1184,6 +1182,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter 
*adapter)
if (adapter->tx_scrq[i]->irq) {
netdev_dbg(netdev,
   "Disabling tx_scrq[%d] irq\n", i);
+   disable_scrq_irq(adapter, adapter->tx_scrq[i]);
disable_irq(adapter->tx_scrq[i]->irq);
}
}
@@ -1193,6 +1192,7 @@ static void ibmvnic_disable_irqs(struct ibmvnic_adapter 
*adapter)
if (adapter->rx_scrq[i]->irq) {
netdev_dbg(netdev,
   "Disabling rx_scrq[%d] irq\n", i);
+   disable_scrq_irq(adapter, adapter->rx_scrq[i]);
disable_irq(adapter->rx_scrq[i]->irq);
}
}
@@ -2601,12 +2601,19 @@ static int enable_scrq_irq(struct ibmvnic_adapter 
*adapter,
 {
struct device *dev = >vdev->dev;
unsigned long rc;
+   u64 val;
 
if (scrq->hw_irq > 0x1ULL) {
dev_err(dev, "bad hw_irq = %lx\n", scrq->hw_irq);
return 1;
}
 
+   val = (0xff00) | scrq->hw_irq;
+   rc = plpar_hcall_norets(H_EOI, val);
+   if (rc)
+   dev_err(dev, "H_EOI FAILED irq 0x%llx. rc=%ld\n",
+   val, rc);
+
rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
H_ENABLE_VIO_INTERRUPT, scrq->hw_irq, 0, 0);
if (rc)
-- 
1.8.3.1

Re: [PATCH net-next] ibmvnic: Potential NULL dereference in clean_one_tx_pool()

[Thomas Falcon]

On 03/23/2018 06:36 AM, Dan Carpenter wrote:
> There is an && vs || typo here, which potentially leads to a NULL
> dereference.

Thanks for catching that!

>
> Fixes: e9e1e97884b7 ("ibmvnic: Update TX pool cleaning routine")
> Signed-off-by: Dan Carpenter 
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index 5632c030811b..0389a7a52152 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -1135,7 +1135,7 @@ static void clean_one_tx_pool(struct ibmvnic_adapter 
> *adapter,
>   u64 tx_entries;
>   int i;
>
> - if (!tx_pool && !tx_pool->tx_buff)
> + if (!tx_pool || !tx_pool->tx_buff)
>   return;
>
>   tx_entries = tx_pool->num_buffers;
>

[PATCH net 2/2] ibmvnic: Fix non-atomic memory allocation in IRQ context

[Thomas Falcon]

ibmvnic_reset allocated new reset work item objects in a non-atomic
context. This can be called from a tasklet, generating the output below.
Allocate work items with the GFP_ATOMIC flag instead.

BUG: sleeping function called from invalid context at mm/slab.h:421
in_atomic(): 1, irqs_disabled(): 1, pid: 93, name: kworker/0:2
INFO: lockdep is turned off.
irq event stamp: 66049
hardirqs last  enabled at (66048): [] 
tasklet_action_common.isra.12+0x78/0x1c0
hardirqs last disabled at (66049): [] 
_raw_spin_lock_irqsave+0x48/0xf0
softirqs last  enabled at (66044): [] 
dev_deactivate_queue.constprop.28+0xc8/0x160
softirqs last disabled at (66045): [] 
call_do_softirq+0x14/0x24
CPU: 0 PID: 93 Comm: kworker/0:2 Kdump: loaded Not tainted 
4.20.0-rc6-1-g1b50a8f03706 #7
Workqueue: events linkwatch_event
Call Trace:
[c003fffe7ae0] [c0bc83e4] dump_stack+0xe8/0x164 (unreliable)
[c003fffe7b30] [c015ba0c] ___might_sleep+0x2dc/0x320
[c003fffe7bb0] [c0391514] kmem_cache_alloc_trace+0x3e4/0x440
[c003fffe7c30] [d5b2309c] ibmvnic_reset+0x16c/0x360 [ibmvnic]
[c003fffe7cc0] [d5b29834] ibmvnic_tasklet+0x1054/0x2010 [ibmvnic]
[c003fffe7e00] [c01224c8] tasklet_action_common.isra.12+0xd8/0x1c0
[c003fffe7e60] [c0bf1238] __do_softirq+0x1a8/0x64c
[c003fffe7f90] [c00306e0] call_do_softirq+0x14/0x24
[c003f3967980] [c001ba50] do_softirq_own_stack+0x60/0xb0
[c003f39679c0] [c01218a8] do_softirq+0xa8/0x100
[c003f39679f0] [c0121a74] __local_bh_enable_ip+0x174/0x180
[c003f3967a60] [c0bf003c] _raw_spin_unlock_bh+0x5c/0x80
[c003f3967a90] [c0a8ac78] 
dev_deactivate_queue.constprop.28+0xc8/0x160
[c003f3967ad0] [c0a8c8b0] dev_deactivate_many+0xd0/0x520
[c003f3967b70] [c0a8cd40] dev_deactivate+0x40/0x60
[c003f3967ba0] [c0a5e0c4] linkwatch_do_dev+0x74/0xd0
[c003f3967bd0] [c0a5e694] __linkwatch_run_queue+0x1a4/0x1f0
[c003f3967c30] [c0a5e728] linkwatch_event+0x48/0x60
[c003f3967c50] [c01444e8] process_one_work+0x238/0x710
[c003f3967d20] [c0144a48] worker_thread+0x88/0x4e0
[c003f3967db0] [c014e3a8] kthread+0x178/0x1c0
[c003f3967e20] [c000bfd0] ret_from_kernel_thread+0x5c/0x6c

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index ffc0cab05b0f..67cc6d9c8fd7 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2055,7 +2055,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
}
}
 
-   rwi = kzalloc(sizeof(*rwi), GFP_KERNEL);
+   rwi = kzalloc(sizeof(*rwi), GFP_ATOMIC);
if (!rwi) {
spin_unlock_irqrestore(>rwi_lock, flags);
ibmvnic_close(netdev);
-- 
2.12.3

[PATCH net 1/2] ibmvnic: Convert reset work item mutex to spin lock

[Thomas Falcon]

ibmvnic_reset can create and schedule a reset work item from
an IRQ context, so do not use a mutex, which can sleep. Convert
the reset work item mutex to a spin lock. Locking debugger generated
the trace output below.

BUG: sleeping function called from invalid context at kernel/locking/mutex.c:908
in_atomic(): 1, irqs_disabled(): 1, pid: 120, name: kworker/8:1
4 locks held by kworker/8:1/120:
 #0: 17c05720 ((wq_completion)"events"){+.+.}, at: 
process_one_work+0x188/0x710
 #1: ace90706 ((linkwatch_work).work){+.+.}, at: 
process_one_work+0x188/0x710
 #2: 7632871f (rtnl_mutex){+.+.}, at: rtnl_lock+0x30/0x50
 #3: fc36813a (&(>lock)->rlock){..-.}, at: 
ibmvnic_tasklet+0x88/0x2010 [ibmvnic]
irq event stamp: 26293
hardirqs last  enabled at (26292): [] 
tasklet_action_common.isra.12+0x78/0x1c0
hardirqs last disabled at (26293): [] 
_raw_spin_lock_irqsave+0x48/0xf0
softirqs last  enabled at (26288): [] 
dev_deactivate_queue.constprop.28+0xc8/0x160
softirqs last disabled at (26289): [] 
call_do_softirq+0x14/0x24
CPU: 8 PID: 120 Comm: kworker/8:1 Kdump: loaded Not tainted 4.20.0-rc6 #6
Workqueue: events linkwatch_event
Call Trace:
[c003fffa7a50] [c0bc83e4] dump_stack+0xe8/0x164 (unreliable)
[c003fffa7aa0] [c015ba0c] ___might_sleep+0x2dc/0x320
[c003fffa7b20] [c0be960c] __mutex_lock+0x8c/0xb40
[c003fffa7c30] [d6202ac8] ibmvnic_reset+0x78/0x330 [ibmvnic]
[c003fffa7cc0] [d62097f4] ibmvnic_tasklet+0x1054/0x2010 [ibmvnic]
[c003fffa7e00] [c01224c8] tasklet_action_common.isra.12+0xd8/0x1c0
[c003fffa7e60] [c0bf1238] __do_softirq+0x1a8/0x64c
[c003fffa7f90] [c00306e0] call_do_softirq+0x14/0x24
[c003f3f87980] [c001ba50] do_softirq_own_stack+0x60/0xb0
[c003f3f879c0] [c01218a8] do_softirq+0xa8/0x100
[c003f3f879f0] [c0121a74] __local_bh_enable_ip+0x174/0x180
[c003f3f87a60] [c0bf003c] _raw_spin_unlock_bh+0x5c/0x80
[c003f3f87a90] [c0a8ac78] 
dev_deactivate_queue.constprop.28+0xc8/0x160
[c003f3f87ad0] [c0a8c8b0] dev_deactivate_many+0xd0/0x520
[c003f3f87b70] [c0a8cd40] dev_deactivate+0x40/0x60
[c003f3f87ba0] [c0a5e0c4] linkwatch_do_dev+0x74/0xd0
[c003f3f87bd0] [c0a5e694] __linkwatch_run_queue+0x1a4/0x1f0
[c003f3f87c30] [c0a5e728] linkwatch_event+0x48/0x60
[c003f3f87c50] [c01444e8] process_one_work+0x238/0x710
[c003f3f87d20] [c0144a48] worker_thread+0x88/0x4e0
[c003f3f87db0] [c014e3a8] kthread+0x178/0x1c0
[c003f3f87e20] [c000bfd0] ret_from_kernel_thread+0x5c/0x6c

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 16 +---
 drivers/net/ethernet/ibm/ibmvnic.h |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index ed50b8dee44f..ffc0cab05b0f 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1939,8 +1939,9 @@ static int do_hard_reset(struct ibmvnic_adapter *adapter,
 static struct ibmvnic_rwi *get_next_rwi(struct ibmvnic_adapter *adapter)
 {
struct ibmvnic_rwi *rwi;
+   unsigned long flags;
 
-   mutex_lock(>rwi_lock);
+   spin_lock_irqsave(>rwi_lock, flags);
 
if (!list_empty(>rwi_list)) {
rwi = list_first_entry(>rwi_list, struct ibmvnic_rwi,
@@ -1950,7 +1951,7 @@ static struct ibmvnic_rwi *get_next_rwi(struct 
ibmvnic_adapter *adapter)
rwi = NULL;
}
 
-   mutex_unlock(>rwi_lock);
+   spin_unlock_irqrestore(>rwi_lock, flags);
return rwi;
 }
 
@@ -2025,6 +2026,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
struct list_head *entry, *tmp_entry;
struct ibmvnic_rwi *rwi, *tmp;
struct net_device *netdev = adapter->netdev;
+   unsigned long flags;
int ret;
 
if (adapter->state == VNIC_REMOVING ||
@@ -2041,13 +2043,13 @@ static int ibmvnic_reset(struct ibmvnic_adapter 
*adapter,
goto err;
}
 
-   mutex_lock(>rwi_lock);
+   spin_lock_irqsave(>rwi_lock, flags);
 
list_for_each(entry, >rwi_list) {
tmp = list_entry(entry, struct ibmvnic_rwi, list);
if (tmp->reset_reason == reason) {
netdev_dbg(netdev, "Skipping matching reset\n");
-   mutex_unlock(>rwi_lock);
+   spin_unlock_irqrestore(>rwi_lock, flags);
ret = EBUSY;
goto err;
}
@@ -2055,7 +2057,7 @@ static int ibmvnic_reset(struct ibmvnic_adapter *adapter,
 
rwi = kzalloc(sizeof(*rwi), GFP_KERNEL);
if (!rwi) {
-   mutex_unlock(>rwi_lock);
+   spin_unlock_irqrestore(>rwi_loc

[PATCH net 0/2] net/ibmvnic: Fix reset work item locking bugs

[Thomas Falcon]

This patch set fixes issues with scheduling reset work items in
a tasklet context. Since ibmvnic_reset can called in an interrupt,
it should not use a mutex or allocate memory non-atomically.

Thomas Falcon (2):
  ibmvnic: Convert reset work item mutex to spin lock
  ibmvnic: Fix non-atomic memory allocation in IRQ context

 drivers/net/ethernet/ibm/ibmvnic.c | 18 ++
 drivers/net/ethernet/ibm/ibmvnic.h |  2 +-
 2 files changed, 11 insertions(+), 9 deletions(-)

-- 
2.12.3

[PATCH net 1/2] ibmvnic: Fix RX queue buffer cleanup

[Thomas Falcon]

The wrong index is used when cleaning up RX buffer objects during release
of RX queues. Update to use the correct index counter.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 27a6df3..066897a 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -485,8 +485,8 @@ static void release_rx_pools(struct ibmvnic_adapter 
*adapter)
 
for (j = 0; j < rx_pool->size; j++) {
if (rx_pool->rx_buff[j].skb) {
-   dev_kfree_skb_any(rx_pool->rx_buff[i].skb);
-   rx_pool->rx_buff[i].skb = NULL;
+   dev_kfree_skb_any(rx_pool->rx_buff[j].skb);
+   rx_pool->rx_buff[j].skb = NULL;
}
}
 
-- 
1.8.3.1

[PATCH net 0/2] ibmvnic: Fix queue and buffer accounting errors

[Thomas Falcon]

This series includes two small fixes. The first resolves a typo bug
in the code to clean up unused RX buffers during device queue removal.
The second ensures that device queue memory is updated to reflect new
supported queue ring sizes after migration to other backing hardware.


Thomas Falcon (2):
  ibmvnic: Fix RX queue buffer cleanup
  ibmvnic: Update driver queues after change in ring size support

 drivers/net/ethernet/ibm/ibmvnic.c | 13 ++---
 1 file changed, 10 insertions(+), 3 deletions(-)

-- 
1.8.3.1

[PATCH net 2/2] ibmvnic: Update driver queues after change in ring size support

[Thomas Falcon]

During device reset, queue memory is not being updated to accommodate
changes in ring buffer sizes supported by backing hardware. Track
any differences in ring buffer sizes following the reset and update
queue memory when possible.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 066897a..c0203a0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1737,6 +1737,7 @@ static int do_reset(struct ibmvnic_adapter *adapter,
struct ibmvnic_rwi *rwi, u32 reset_state)
 {
u64 old_num_rx_queues, old_num_tx_queues;
+   u64 old_num_rx_slots, old_num_tx_slots;
struct net_device *netdev = adapter->netdev;
int i, rc;
 
@@ -1748,6 +1749,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 
old_num_rx_queues = adapter->req_rx_queues;
old_num_tx_queues = adapter->req_tx_queues;
+   old_num_rx_slots = adapter->req_rx_add_entries_per_subcrq;
+   old_num_tx_slots = adapter->req_tx_entries_per_subcrq;
 
ibmvnic_cleanup(netdev);
 
@@ -1810,7 +1813,11 @@ static int do_reset(struct ibmvnic_adapter *adapter,
if (rc)
return rc;
} else if (adapter->req_rx_queues != old_num_rx_queues ||
-  adapter->req_tx_queues != old_num_tx_queues) {
+  adapter->req_tx_queues != old_num_tx_queues ||
+  adapter->req_rx_add_entries_per_subcrq !=
+   old_num_rx_slots ||
+  adapter->req_tx_entries_per_subcrq !=
+   old_num_tx_slots) {
release_rx_pools(adapter);
release_tx_pools(adapter);
release_napi(adapter);
-- 
1.8.3.1

[PATCH net 1/3] ibmvnic: Do not close unopened driver during reset

[Thomas Falcon]

Check driver state before halting it during a reset. If the driver is
not running, do nothing. Otherwise, a request to deactivate a down link
can cause an error and the reset will fail.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 3da392b..bc2a912 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1745,7 +1745,8 @@ static int do_reset(struct ibmvnic_adapter *adapter,
 
ibmvnic_cleanup(netdev);
 
-   if (adapter->reset_reason != VNIC_RESET_MOBILITY &&
+   if (reset_state == VNIC_OPEN &&
+   adapter->reset_reason != VNIC_RESET_MOBILITY &&
adapter->reset_reason != VNIC_RESET_FAILOVER) {
rc = __ibmvnic_close(netdev);
if (rc)
-- 
1.8.3.1

[PATCH net 2/3] ibmvnic: Refresh device multicast list after reset

[Thomas Falcon]

It was observed that multicast packets were no longer received after
a device reset.  The fix is to resend the current multicast list to
the backing device after recovery.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index bc2a912..9e9f409 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1845,6 +1845,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
return 0;
}
 
+   /* refresh device's multicast list */
+   ibmvnic_set_multi(netdev);
+
/* kick napi */
for (i = 0; i < adapter->req_rx_queues; i++)
napi_schedule(>napi[i]);
-- 
1.8.3.1

[PATCH net 3/3] ibmvnic: Fix unchecked return codes of memory allocations

[Thomas Falcon]

The return values for these memory allocations are unchecked,
which may cause an oops if the driver does not handle them after
a failure. Fix by checking the function's return code.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9e9f409..3da6800 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -428,9 +428,10 @@ static int reset_rx_pools(struct ibmvnic_adapter *adapter)
if (rx_pool->buff_size != be64_to_cpu(size_array[i])) {
free_long_term_buff(adapter, _pool->long_term_buff);
rx_pool->buff_size = be64_to_cpu(size_array[i]);
-   alloc_long_term_buff(adapter, _pool->long_term_buff,
-rx_pool->size *
-rx_pool->buff_size);
+   rc = alloc_long_term_buff(adapter,
+ _pool->long_term_buff,
+ rx_pool->size *
+ rx_pool->buff_size);
} else {
rc = reset_long_term_buff(adapter,
  _pool->long_term_buff);
@@ -696,9 +697,9 @@ static int init_tx_pools(struct net_device *netdev)
return rc;
}
 
-   init_one_tx_pool(netdev, >tso_pool[i],
-IBMVNIC_TSO_BUFS,
-IBMVNIC_TSO_BUF_SZ);
+   rc = init_one_tx_pool(netdev, >tso_pool[i],
+ IBMVNIC_TSO_BUFS,
+ IBMVNIC_TSO_BUF_SZ);
if (rc) {
release_tx_pools(adapter);
return rc;
-- 
1.8.3.1

[PATCH net 0/3] ibmvnic: Fixes for device reset handling

[Thomas Falcon]

This series contains three unrelated fixes to issues seen during
device resets. The first patch fixes an error when the driver requests
to deactivate the link of an uninitialized device, resulting in a 
failure to reset. Next, a patch to fix multicast transmission 
failures seen after a driver reset. The final patch fixes mishandling
of memory allocation failures during device initialization, which
caused a kernel oops.

Thomas Falcon (3):
  ibmvnic: Do not close unopened driver during reset
  ibmvnic: Refresh device multicast list after reset
  ibmvnic: Fix unchecked return codes of memory allocations

 drivers/net/ethernet/ibm/ibmvnic.c | 19 ---
 1 file changed, 12 insertions(+), 7 deletions(-)

-- 
1.8.3.1

Re: [PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool

[Thomas Falcon]

On 6/27/19 12:57 PM, Andrew Lunn wrote:

On Thu, Jun 27, 2019 at 12:09:13PM -0500, Thomas Falcon wrote:

This patch resolves an issue with sensitive bonding modes
that require valid speed and duplex settings to function
properly. Currently, the adapter will report that device
speed and duplex is unknown if the communication link
with firmware is unavailable.

Dumb question. If you cannot communicate with the firmware, isn't the
device FUBAR? So setting the LACP port to disabled is the correct
things to do.

Andrew

Yes, I think that is correct too.  The problem is that the link is only 
down temporarily.  In this case - we are testing with a pseries logical 
partition - the partition is migrated to another server. The driver must 
wait for a signal from the hypervisor to resume operation with the new 
device.  Once it resumes, we see that the device reboots and gets 
correct speed settings, but the port flag (AD_LACP_PORT_ENABLED) is 
still cleared.


Tom

[PATCH net] net/ibmvnic: Report last valid speed and duplex values to ethtool

[Thomas Falcon]

This patch resolves an issue with sensitive bonding modes
that require valid speed and duplex settings to function
properly. Currently, the adapter will report that device
speed and duplex is unknown if the communication link
with firmware is unavailable. This decision can break LACP
configurations if the timing is right.

For example, if invalid speeds are reported, the slave
device's link state is set to a transitional "fail" state
and the LACP port is disabled. However, if valid speeds
are reported later but the link state has not been altered,
the LACP port will remain disabled. If the link state then
transitions back to "up" from "fail," it results in a state
such that the slave reports valid speed/duplex and is up,
but the LACP port will remain disabled.

Workaround this by reporting the last recorded speed
and duplex settings unless the device has never been
activated. In that case or when the hypervisor gives
invalid values, continue to report unknown speed or
duplex to ethtool.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 3da6800..7c14e33 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -2276,10 +2276,8 @@ static int ibmvnic_get_link_ksettings(struct net_device 
*netdev,
int rc;
 
rc = send_query_phys_parms(adapter);
-   if (rc) {
-   adapter->speed = SPEED_UNKNOWN;
-   adapter->duplex = DUPLEX_UNKNOWN;
-   }
+   if (rc)
+   netdev_warn(netdev, "Device query of current speed and duplex 
settings failed; reported values may be stale.\n");
cmd->base.speed = adapter->speed;
cmd->base.duplex = adapter->duplex;
cmd->base.port = PORT_FIBRE;
@@ -4834,6 +4832,8 @@ static int ibmvnic_probe(struct vio_dev *dev, const 
struct vio_device_id *id)
dev_set_drvdata(>dev, netdev);
adapter->vdev = dev;
adapter->netdev = netdev;
+   adapter->speed = SPEED_UNKNOWN;
+   adapter->duplex = DUPLEX_UNKNOWN;
 
ether_addr_copy(adapter->mac_addr, mac_addr_p);
ether_addr_copy(netdev->dev_addr, adapter->mac_addr);
-- 
1.8.3.1

[PATCH net] ibmvnic: Do not process reset during or after device removal

[Thomas Falcon]

Currently, the ibmvnic driver will not schedule device resets
if the device is being removed, but does not check the device
state before the reset is actually processed. This leads to a race
where a reset is scheduled with a valid device state but is
processed after the driver has been removed, resulting in an oops.

Fix this by checking the device state before processing a queued
reset event.

Reported-by: Abdul Haleem 
Tested-by: Abdul Haleem 
Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index cebd20f..fa4bb94 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1983,6 +1983,10 @@ static void __ibmvnic_reset(struct work_struct *work)
 
rwi = get_next_rwi(adapter);
while (rwi) {
+   if (adapter->state == VNIC_REMOVING ||
+   adapter->state == VNIC_REMOVED)
+   goto out;
+
if (adapter->force_reset_recovery) {
adapter->force_reset_recovery = false;
rc = do_hard_reset(adapter, rwi, reset_state);
@@ -2007,7 +2011,7 @@ static void __ibmvnic_reset(struct work_struct *work)
netdev_dbg(adapter->netdev, "Reset failed\n");
free_all_rwi(adapter);
}
-
+out:
adapter->resetting = false;
if (we_lock_rtnl)
rtnl_unlock();
-- 
1.8.3.1

Re: [PATCH net-next] ibmveth: Allow users to update reported speed and duplex

[Thomas Falcon]

On 8/6/19 5:25 AM, Michael Ellerman wrote:

Thomas Falcon  writes:

Reported ethtool link settings for the ibmveth driver are currently
hardcoded and no longer reflect the actual capabilities of supported
hardware. There is no interface designed for retrieving this information
from device firmware nor is there any way to update current settings
to reflect observed or expected link speeds.

To avoid confusion, initially define speed and duplex as unknown and

Doesn't that risk break existing setups?



You're right, sorry for missing that.





allow the user to alter these settings to match the expected
capabilities of underlying hardware if needed. This update would allow
the use of configurations that rely on certain link speed settings,
such as LACP. This patch is based on the implementation in virtio_net.

Wouldn't it be safer to keep the current values as the default, and then
also allow them to be overridden by a motivated user.


That is a good compromise.  I will resend an updated version soon with 
that change.


Thanks!




cheers

[PATCH net-next] ibmveth: Allow users to update reported speed and duplex

[Thomas Falcon]

Reported ethtool link settings for the ibmveth driver are currently
hardcoded and no longer reflect the actual capabilities of supported
hardware. There is no interface designed for retrieving this information
from device firmware nor is there any way to update current settings
to reflect observed or expected link speeds.

To avoid confusion, initially define speed and duplex as unknown and
allow the user to alter these settings to match the expected
capabilities of underlying hardware if needed. This update would allow
the use of configurations that rely on certain link speed settings,
such as LACP. This patch is based on the implementation in virtio_net.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmveth.c | 83 --
 drivers/net/ethernet/ibm/ibmveth.h |  3 ++
 2 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index d654c23..77af9c2 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -712,31 +712,68 @@ static int ibmveth_close(struct net_device *netdev)
return 0;
 }
 
-static int netdev_get_link_ksettings(struct net_device *dev,
-struct ethtool_link_ksettings *cmd)
+static bool
+ibmveth_validate_ethtool_cmd(const struct ethtool_link_ksettings *cmd)
 {
-   u32 supported, advertising;
-
-   supported = (SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg |
-   SUPPORTED_FIBRE);
-   advertising = (ADVERTISED_1000baseT_Full | ADVERTISED_Autoneg |
-   ADVERTISED_FIBRE);
-   cmd->base.speed = SPEED_1000;
-   cmd->base.duplex = DUPLEX_FULL;
-   cmd->base.port = PORT_FIBRE;
-   cmd->base.phy_address = 0;
-   cmd->base.autoneg = AUTONEG_ENABLE;
-
-   ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
-   supported);
-   ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
-   advertising);
+   struct ethtool_link_ksettings diff1 = *cmd;
+   struct ethtool_link_ksettings diff2 = {};
+
+   diff2.base.port = PORT_OTHER;
+   diff1.base.speed = 0;
+   diff1.base.duplex = 0;
+   diff1.base.cmd = 0;
+   diff1.base.link_mode_masks_nwords = 0;
+   ethtool_link_ksettings_zero_link_mode(, advertising);
+
+   return !memcmp(, , sizeof(diff1.base)) &&
+   bitmap_empty(diff1.link_modes.supported,
+__ETHTOOL_LINK_MODE_MASK_NBITS) &&
+   bitmap_empty(diff1.link_modes.advertising,
+__ETHTOOL_LINK_MODE_MASK_NBITS) &&
+   bitmap_empty(diff1.link_modes.lp_advertising,
+__ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static int ibmveth_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   u32 speed;
+   u8 duplex;
+
+   speed = cmd->base.speed;
+   duplex = cmd->base.duplex;
+   /* don't allow custom speed and duplex */
+   if (!ethtool_validate_speed(speed) ||
+   !ethtool_validate_duplex(duplex) ||
+   !ibmveth_validate_ethtool_cmd(cmd))
+   return -EINVAL;
+   adapter->speed = speed;
+   adapter->duplex = duplex;
 
return 0;
 }
 
-static void netdev_get_drvinfo(struct net_device *dev,
-  struct ethtool_drvinfo *info)
+static int ibmveth_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+
+   cmd->base.speed = adapter->speed;
+   cmd->base.duplex = adapter->duplex;
+   cmd->base.port = PORT_OTHER;
+
+   return 0;
+}
+
+static void ibmveth_init_link_settings(struct ibmveth_adapter *adapter)
+{
+   adapter->duplex = DUPLEX_UNKNOWN;
+   adapter->speed = SPEED_UNKNOWN;
+}
+
+static void ibmveth_get_drvinfo(struct net_device *dev,
+   struct ethtool_drvinfo *info)
 {
strlcpy(info->driver, ibmveth_driver_name, sizeof(info->driver));
strlcpy(info->version, ibmveth_driver_version, sizeof(info->version));
@@ -965,12 +1002,13 @@ static void ibmveth_get_ethtool_stats(struct net_device 
*dev,
 }
 
 static const struct ethtool_ops netdev_ethtool_ops = {
-   .get_drvinfo= netdev_get_drvinfo,
+   .get_drvinfo= ibmveth_get_drvinfo,
.get_link   = ethtool_op_get_link,
.get_strings= ibmveth_get_strings,
.get_sset_count = ibmveth_get_sset_count,
.get_ethtool_stats

[PATCH net-next v2] ibmveth: Allow users to update reported speed and duplex

[Thomas Falcon]

Reported ethtool link settings for the ibmveth driver are currently
hardcoded and no longer reflect the actual capabilities of supported
hardware. There is no interface designed for retrieving this information
from device firmware nor is there any way to update current settings
to reflect observed or expected link speeds.

To avoid breaking existing configurations, retain current values as
default settings but let users update them to match the expected
capabilities of underlying hardware if needed. This update would
allow the use of configurations that rely on certain link speed
settings, such as LACP. This patch is based on the implementation
in virtio_net.

Signed-off-by: Thomas Falcon 
---
v2: Updated default driver speed/duplex settings to avoid
breaking existing setups
---
 drivers/net/ethernet/ibm/ibmveth.c | 83 --
 drivers/net/ethernet/ibm/ibmveth.h |  3 ++
 2 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmveth.c 
b/drivers/net/ethernet/ibm/ibmveth.c
index d654c23..5dc634f 100644
--- a/drivers/net/ethernet/ibm/ibmveth.c
+++ b/drivers/net/ethernet/ibm/ibmveth.c
@@ -712,31 +712,68 @@ static int ibmveth_close(struct net_device *netdev)
return 0;
 }
 
-static int netdev_get_link_ksettings(struct net_device *dev,
-struct ethtool_link_ksettings *cmd)
+static bool
+ibmveth_validate_ethtool_cmd(const struct ethtool_link_ksettings *cmd)
 {
-   u32 supported, advertising;
-
-   supported = (SUPPORTED_1000baseT_Full | SUPPORTED_Autoneg |
-   SUPPORTED_FIBRE);
-   advertising = (ADVERTISED_1000baseT_Full | ADVERTISED_Autoneg |
-   ADVERTISED_FIBRE);
-   cmd->base.speed = SPEED_1000;
-   cmd->base.duplex = DUPLEX_FULL;
-   cmd->base.port = PORT_FIBRE;
-   cmd->base.phy_address = 0;
-   cmd->base.autoneg = AUTONEG_ENABLE;
-
-   ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.supported,
-   supported);
-   ethtool_convert_legacy_u32_to_link_mode(cmd->link_modes.advertising,
-   advertising);
+   struct ethtool_link_ksettings diff1 = *cmd;
+   struct ethtool_link_ksettings diff2 = {};
+
+   diff2.base.port = PORT_OTHER;
+   diff1.base.speed = 0;
+   diff1.base.duplex = 0;
+   diff1.base.cmd = 0;
+   diff1.base.link_mode_masks_nwords = 0;
+   ethtool_link_ksettings_zero_link_mode(, advertising);
+
+   return !memcmp(, , sizeof(diff1.base)) &&
+   bitmap_empty(diff1.link_modes.supported,
+__ETHTOOL_LINK_MODE_MASK_NBITS) &&
+   bitmap_empty(diff1.link_modes.advertising,
+__ETHTOOL_LINK_MODE_MASK_NBITS) &&
+   bitmap_empty(diff1.link_modes.lp_advertising,
+__ETHTOOL_LINK_MODE_MASK_NBITS);
+}
+
+static int ibmveth_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+   u32 speed;
+   u8 duplex;
+
+   speed = cmd->base.speed;
+   duplex = cmd->base.duplex;
+   /* don't allow custom speed and duplex */
+   if (!ethtool_validate_speed(speed) ||
+   !ethtool_validate_duplex(duplex) ||
+   !ibmveth_validate_ethtool_cmd(cmd))
+   return -EINVAL;
+   adapter->speed = speed;
+   adapter->duplex = duplex;
 
return 0;
 }
 
-static void netdev_get_drvinfo(struct net_device *dev,
-  struct ethtool_drvinfo *info)
+static int ibmveth_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+{
+   struct ibmveth_adapter *adapter = netdev_priv(dev);
+
+   cmd->base.speed = adapter->speed;
+   cmd->base.duplex = adapter->duplex;
+   cmd->base.port = PORT_OTHER;
+
+   return 0;
+}
+
+static void ibmveth_init_link_settings(struct ibmveth_adapter *adapter)
+{
+   adapter->duplex = DUPLEX_FULL;
+   adapter->speed = SPEED_1000;
+}
+
+static void ibmveth_get_drvinfo(struct net_device *dev,
+   struct ethtool_drvinfo *info)
 {
strlcpy(info->driver, ibmveth_driver_name, sizeof(info->driver));
strlcpy(info->version, ibmveth_driver_version, sizeof(info->version));
@@ -965,12 +1002,13 @@ static void ibmveth_get_ethtool_stats(struct net_device 
*dev,
 }
 
 static const struct ethtool_ops netdev_ethtool_ops = {
-   .get_drvinfo= netdev_get_drvinfo,
+   .get_drvinfo= ibmveth_get_drvinfo,
.get_link   = ethtool_op_get_link,
.get_strings= ibmveth_get_str

[RFC PATCH] powerpc/pseries/mobility: notify network peers after migration

[Thomas Falcon]

After a migration, it is necessary to send a gratuitous ARP
from all running interfaces so that the rest of the network
is aware of its new location. However, some supported network
devices are unaware that they have been migrated. To avoid network
interruptions and other unwanted behavior, force a GARP on all
valid, running interfaces as part of the post_mobility_fixup
routine.

Signed-off-by: Thomas Falcon 
---
 arch/powerpc/platforms/pseries/mobility.c | 20 
 1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/mobility.c 
b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..c1abc14cf2bb 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -17,6 +17,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -331,6 +334,8 @@ void post_mobility_fixup(void)
 {
int rc;
int activate_fw_token;
+   struct net_device *netdev;
+   struct net *net;
 
activate_fw_token = rtas_token("ibm,activate-firmware");
if (activate_fw_token == RTAS_UNKNOWN_SERVICE) {
@@ -371,6 +376,21 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
 
+   /* need to force a gratuitous ARP on running interfaces */
+   rtnl_lock();
+   for_each_net(net) {
+   for_each_netdev(net, netdev) {
+   if (netif_device_present(netdev) &&
+   netif_running(netdev) &&
+   !(netdev->flags & (IFF_NOARP | IFF_LOOPBACK)))
+   call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+netdev);
+   call_netdevice_notifiers(NETDEV_RESEND_IGMP,
+netdev);
+   }
+   }
+   rtnl_unlock();
+
return;
 }
 
-- 
2.12.3

Re: [RFC PATCH] powerpc/pseries/mobility: notify network peers after migration

[Thomas Falcon]

On 11/5/19 10:13 PM, Russell Currey wrote:

On Tue, 2019-11-05 at 18:06 -0600, Thomas Falcon wrote:

After a migration, it is necessary to send a gratuitous ARP
from all running interfaces so that the rest of the network
is aware of its new location. However, some supported network
devices are unaware that they have been migrated. To avoid network
interruptions and other unwanted behavior, force a GARP on all
valid, running interfaces as part of the post_mobility_fixup
routine.

Signed-off-by: Thomas Falcon 

Hi Thomas,


---
  arch/powerpc/platforms/pseries/mobility.c | 20 
  1 file changed, 20 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/mobility.c
b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..c1abc14cf2bb 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -17,6 +17,9 @@
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 
  
  #include 

  #include 
@@ -331,6 +334,8 @@ void post_mobility_fixup(void)
  {
int rc;
int activate_fw_token;
+   struct net_device *netdev;
+   struct net *net;
  
  	activate_fw_token = rtas_token("ibm,activate-firmware");

if (activate_fw_token == RTAS_UNKNOWN_SERVICE) {
@@ -371,6 +376,21 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
  
+	/* need to force a gratuitous ARP on running interfaces */

+   rtnl_lock();
+   for_each_net(net) {
+   for_each_netdev(net, netdev) {
+   if (netif_device_present(netdev) &&
+   netif_running(netdev) &&
+   !(netdev->flags & (IFF_NOARP |
IFF_LOOPBACK)))
+   call_netdevice_notifiers(NETDEV_NOTIFY_
PEERS,
+netdev);

Without curly braces following the "if" statment, the second line
(below) will be executed unconditionally, which I assume with this
indentation isn't what you want.

(reported by snowpatch)

- Russell


Thanks for catching that! I'll fix that and send a v2 soon.

Tom



+   call_netdevice_notifiers(NETDEV_RESEND_
IGMP,
+netdev);
+   }
+   }
+   rtnl_unlock();
+
return;
  }
  

Re: [PATCH 0/9] Fixes and Enablement of ibm,drc-info property

[Thomas Falcon]

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

Hi, just pointing out a few typos...

There was a previous effort to add support for the PAPR
architected ibm,drc-info property. This property provides a more
memory compact representation of a paritions Dynamic Reconfig

s/paritions/partition's

Connectors (DRC). These can otherwise be thought of as currently
partitioned, or available but yet to be partitioned system resources
such as cpus, memory, and physical/logical IOA devices.

The initial implementation proved buggy and was fully turned of by


s/turned of/turned off


disabling the bit in the appropriate CAS support vector. We now have
PowerVM firmware in the field that supports this new property, and
further to suppport partitions with 24TB+ of possible memory this

s/suppport/support

property is required to perform platform migration.

This serious fixs the short comings of the previous submission


Either "seriously fixes the shortcomings", or "fixes the serious 
shortcomings?"


Thanks,

Tom


in the areas of general implementation, cpu hotplug, and IOA hotplug.

Tyrel Datwyler (9):
   powerpc/pseries: Fix bad drc_index_start value parsing of drc-info
 entry
   powerpc/pseries: Fix drc-info mappings of logical cpus to drc-index
   powerpc/pseries: Add cpu DLPAR support for drc-info property
   PCI: rpaphp: Fix up pointer to first drc-info entry
   PCI: rpaphp: Don't rely on firmware feature to imply drc-info support
   PCI: rpaphp: Add drc-info support for hotplug slot registration
   PCI: rpaphp: annotate and correctly byte swap DRC properties
   PCI: rpaphp: Correctly match ibm,my-drc-index to drc-name when using
 drc-info
   powerpc/pseries: Enable support for ibm,drc-info property

  arch/powerpc/kernel/prom_init.c |   2 +-
  arch/powerpc/platforms/pseries/hotplug-cpu.c| 101 ---
  arch/powerpc/platforms/pseries/of_helpers.c |   8 +-
  arch/powerpc/platforms/pseries/pseries_energy.c |  23 ++---
  drivers/pci/hotplug/rpaphp_core.c   | 124 +---
  5 files changed, 187 insertions(+), 71 deletions(-)

Re: [PATCH 3/9] powerpc/pseries: Add cpu DLPAR support for drc-info property

[Thomas Falcon]

On 11/5/19 9:24 AM, Tyrel Datwyler wrote:

From: Tyrel Datwyler 

Older firmwares provided information about Dynamic Reconfig
Connectors (DRC) through several device tree properties, namely
ibm,drc-types, ibm,drc-indexes, ibm,drc-names, and
ibm,drc-power-domains. New firmwares have the ability to present this
same information in a much condensed format through a device tree
property called ibm,drc-info.

The existing cpu DLPAR hotplug code only understands the older DRC
property format when validating the drc-index of a cpu during a
hotplug add. This updates those code paths to use the ibm,drc-info
property, when present, instead for validation.

Signed-off-by: Tyrel Datwyler 
---
  arch/powerpc/platforms/pseries/hotplug-cpu.c | 101 ++-
  1 file changed, 85 insertions(+), 16 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-cpu.c 
b/arch/powerpc/platforms/pseries/hotplug-cpu.c
index bbda646..9ba006c 100644
--- a/arch/powerpc/platforms/pseries/hotplug-cpu.c
+++ b/arch/powerpc/platforms/pseries/hotplug-cpu.c
@@ -407,17 +407,58 @@ static bool dlpar_cpu_exists(struct device_node *parent, 
u32 drc_index)
return found;
  }

+static bool drc_info_valid_index(struct device_node *parent, u32 drc_index)
+{
+   struct property *info;
+   struct of_drc_info drc;
+   const __be32 *value;
+   int count, i, j;
+
+   info = of_find_property(parent, "ibm,drc-info", NULL);
+   if (!info)
+   return false;
+
+   value = of_prop_next_u32(info, NULL, );
+
+   /* First value of ibm,drc-info is number of drc-info records */
+   if (value)
+   value++;
+   else
+   return false;
+
+   for (i = 0; i < count; i++) {
+   if (of_read_drc_info_cell(, , ))
+   return false;
+
+   if (strncmp(drc.drc_type, "CPU", 3))
+   break;
+
+   if (drc_index > drc.last_drc_index)
+   continue;
+
+   for (j = 0; j < drc.num_sequential_elems; j++)
+   if (drc_index == (drc.drc_index_start + 
(drc.sequential_inc * j)))
+   return true;
+   }
+
+   return false;
+}
+
  static bool valid_cpu_drc_index(struct device_node *parent, u32 drc_index)
  {
bool found = false;
int rc, index;

-   index = 0;
+   if (of_find_property(parent, "ibm,drc-info", NULL))
+   return drc_info_valid_index(parent, drc_index);
+
+   index = 1;


Hi, this change was confusing to me until I continued reading the patch 
and saw the comment below regarding the first element of the 
ibm,drc-info property.  Would it be good to have a similar comment here too?




while (!found) {
u32 drc;

rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
index++, );
+


Another nitpick but this could be cleaned up.

Thanks,

Tom



if (rc)
break;

@@ -720,8 +761,11 @@ static int dlpar_cpu_remove_by_count(u32 cpus_to_remove)
  static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 cpus_to_add)
  {
struct device_node *parent;
+   struct property *info;
int cpus_found = 0;
int index, rc;
+   int i, j;
+   u32 drc_index;

parent = of_find_node_by_path("/cpus");
if (!parent) {
@@ -730,24 +774,49 @@ static int find_dlpar_cpus_to_add(u32 *cpu_drcs, u32 
cpus_to_add)
return -1;
}

-   /* Search the ibm,drc-indexes array for possible CPU drcs to
-* add. Note that the format of the ibm,drc-indexes array is
-* the number of entries in the array followed by the array
-* of drc values so we start looking at index = 1.
-*/
-   index = 1;
-   while (cpus_found < cpus_to_add) {
-   u32 drc;
+   info = of_find_property(parent, "ibm,drc-info", NULL);
+   if (info) {
+   struct of_drc_info drc;
+   const __be32 *value;
+   int count;

-   rc = of_property_read_u32_index(parent, "ibm,drc-indexes",
-   index++, );
-   if (rc)
-   break;
+   value = of_prop_next_u32(info, NULL, );
+   if (value)
+   value++;

-   if (dlpar_cpu_exists(parent, drc))
-   continue;
+   for (i = 0; i < count; i++) {
+   of_read_drc_info_cell(, , );
+   if (strncmp(drc.drc_type, "CPU", 3))
+   break;
+
+   for (j = 0; j < drc.num_sequential_elems && cpus_found 
< cpus_to_add; j++) {
+   drc_index = drc.drc_index_start + 
(drc.sequential_inc * j);
+
+   if (dlpar_cpu_exists(parent, 

Re: [RFC PATCH] powerpc/pseries/mobility: notify network peers after migration

[Thomas Falcon]

On 11/6/19 7:33 PM, Michael Ellerman wrote:

Hi Thomas,

Thomas Falcon  writes:

After a migration, it is necessary to send a gratuitous ARP
from all running interfaces so that the rest of the network
is aware of its new location. However, some supported network
devices are unaware that they have been migrated. To avoid network
interruptions and other unwanted behavior, force a GARP on all
valid, running interfaces as part of the post_mobility_fixup
routine.

Signed-off-by: Thomas Falcon 
---
  arch/powerpc/platforms/pseries/mobility.c | 20 
  1 file changed, 20 insertions(+)

This patch is in powerpc code, but it's doing networking stuff that I
don't really understand.

So I'd like an Ack from Dave or someone else in netdev land before I
merge it.


Thanks, I've already included netdev in the CC list. I'll wait and keep 
an eye out for any comments from that side.


Tom





cheers



diff --git a/arch/powerpc/platforms/pseries/mobility.c 
b/arch/powerpc/platforms/pseries/mobility.c
index b571285f6c14..c1abc14cf2bb 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -17,6 +17,9 @@
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 
  
  #include 

  #include 
@@ -331,6 +334,8 @@ void post_mobility_fixup(void)
  {
int rc;
int activate_fw_token;
+   struct net_device *netdev;
+   struct net *net;
  
  	activate_fw_token = rtas_token("ibm,activate-firmware");

if (activate_fw_token == RTAS_UNKNOWN_SERVICE) {
@@ -371,6 +376,21 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
  
+	/* need to force a gratuitous ARP on running interfaces */

+   rtnl_lock();
+   for_each_net(net) {
+   for_each_netdev(net, netdev) {
+   if (netif_device_present(netdev) &&
+   netif_running(netdev) &&
+   !(netdev->flags & (IFF_NOARP | IFF_LOOPBACK)))
+   call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+netdev);
+   call_netdevice_notifiers(NETDEV_RESEND_IGMP,
+netdev);
+   }
+   }
+   rtnl_unlock();
+
return;
  }
  
--

2.12.3

Re: [RFC PATCH] powerpc/pseries/mobility: notify network peers after migration

[Thomas Falcon]

On 11/6/19 4:14 PM, Nathan Lynch wrote:

Hi Tom,

Thomas Falcon  writes:

After a migration, it is necessary to send a gratuitous ARP
from all running interfaces so that the rest of the network
is aware of its new location. However, some supported network
devices are unaware that they have been migrated. To avoid network
interruptions and other unwanted behavior, force a GARP on all
valid, running interfaces as part of the post_mobility_fixup
routine.

[...]


@@ -331,6 +334,8 @@ void post_mobility_fixup(void)
  {
int rc;
int activate_fw_token;
+   struct net_device *netdev;
+   struct net *net;
  
  	activate_fw_token = rtas_token("ibm,activate-firmware");

if (activate_fw_token == RTAS_UNKNOWN_SERVICE) {
@@ -371,6 +376,21 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
  
+	/* need to force a gratuitous ARP on running interfaces */

+   rtnl_lock();
+   for_each_net(net) {
+   for_each_netdev(net, netdev) {
+   if (netif_device_present(netdev) &&
+   netif_running(netdev) &&
+   !(netdev->flags & (IFF_NOARP | IFF_LOOPBACK)))
+   call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+netdev);
+   call_netdevice_notifiers(NETDEV_RESEND_IGMP,
+netdev);
+   }
+   }
+   rtnl_unlock();
+

This isn't an outright nak, but this is not nice. It illustrates the
need to rethink the pseries partition migration code. There is no
mechanism for drivers and other interested code to prepare for a
migration or to adjust to the destination. So post_mobility_fixup() will
continue to grow into a fragile collection of calls into unrelated
subsystems until there is a better design -- either a pseries-specific
notification/callback mechanism, or something based on the pm framework.

My understanding is that this is needed specifically for ibmveth and,
unlike ibmvnic, the platform does not provide any notification to that
driver that a migration has occurred, right?


Correct, the ibmveth device, unlike ibmvnic, receives no signal or 
notification at all in the event of a partition migration, so it can not 
handle it or send a gratuitous ARP because from the driver's perspective 
nothing has changed.  As you've described, there is no existing notifier 
in the kernel to inform interested parties that the system has migrated 
or is about to migrate. Without adding the needed infrastructure to do 
that, I'm not sure how else to fix this.


Tom

[RFC PATCH v2] powerpc/pseries/mobility: notify network peers after migration

[Thomas Falcon]

After a migration, it is necessary to send a gratuitous ARP
from all running interfaces so that the rest of the network
is aware of its new location. However, some supported network
devices are unaware that they have been migrated. To avoid network
interruptions and other unwanted behavior, force a GARP on all
valid, running interfaces as part of the post_mobility_fixup
routine.

Signed-off-by: Thomas Falcon 
---
v2: fix missing brackets caught by Russell Currey
---
 arch/powerpc/platforms/pseries/mobility.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/mobility.c 
b/arch/powerpc/platforms/pseries/mobility.c
index b571285..dddc3c1 100644
--- a/arch/powerpc/platforms/pseries/mobility.c
+++ b/arch/powerpc/platforms/pseries/mobility.c
@@ -17,6 +17,9 @@
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
 
 #include 
 #include 
@@ -331,6 +334,8 @@ void post_mobility_fixup(void)
 {
int rc;
int activate_fw_token;
+   struct net_device *netdev;
+   struct net *net;
 
activate_fw_token = rtas_token("ibm,activate-firmware");
if (activate_fw_token == RTAS_UNKNOWN_SERVICE) {
@@ -371,6 +376,22 @@ void post_mobility_fixup(void)
/* Possibly switch to a new RFI flush type */
pseries_setup_rfi_flush();
 
+   /* need to force a gratuitous ARP on running interfaces */
+   rtnl_lock();
+   for_each_net(net) {
+   for_each_netdev(net, netdev) {
+   if (netif_device_present(netdev) &&
+   netif_running(netdev) &&
+   !(netdev->flags & (IFF_NOARP | IFF_LOOPBACK))) {
+   call_netdevice_notifiers(NETDEV_NOTIFY_PEERS,
+netdev);
+   call_netdevice_notifiers(NETDEV_RESEND_IGMP,
+netdev);
+   }
+   }
+   }
+   rtnl_unlock();
+
return;
 }
 
-- 
1.8.3.1

[PATCH net 4/4] ibmvnic: Serialize device queries

[Thomas Falcon]

Provide some serialization for device CRQ commands
and queries to ensure that the shared variable used for
storing return codes is properly synchronized.

Signed-off-by: Thomas Falcon 
---
 drivers/net/ethernet/ibm/ibmvnic.c | 51 ++
 drivers/net/ethernet/ibm/ibmvnic.h |  1 +
 2 files changed, 47 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ibm/ibmvnic.c 
b/drivers/net/ethernet/ibm/ibmvnic.c
index 9806eccc5670..5e9b7711cd2e 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -207,11 +207,14 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
ltb->map_id = adapter->map_id;
adapter->map_id++;
 
+   mutex_lock(>fw_lock);
+   adapter->fw_done_rc = 0;
reinit_completion(>fw_done);
rc = send_request_map(adapter, ltb->addr,
  ltb->size, ltb->map_id);
if (rc) {
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+   mutex_unlock(>fw_lock);
return rc;
}
 
@@ -222,6 +225,7 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
"Long term map request aborted or timed out,rc = %d\n",
rc);
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+   mutex_unlock(>fw_lock);
return rc;
}
 
@@ -229,8 +233,10 @@ static int alloc_long_term_buff(struct ibmvnic_adapter 
*adapter,
dev_err(dev, "Couldn't map long term buffer,rc = %d\n",
adapter->fw_done_rc);
dma_free_coherent(dev, ltb->size, ltb->buff, ltb->addr);
+   mutex_unlock(>fw_lock);
return -1;
}
+   mutex_unlock(>fw_lock);
return 0;
 }
 
@@ -256,16 +262,21 @@ static int reset_long_term_buff(struct ibmvnic_adapter 
*adapter,
 
memset(ltb->buff, 0, ltb->size);
 
+   mutex_lock(>fw_lock);
+   adapter->fw_done_rc = 0;
reinit_completion(>fw_done);
rc = send_request_map(adapter, ltb->addr, ltb->size, ltb->map_id);
-   if (rc)
+   if (rc) {
+   mutex_unlock(>fw_lock);
return rc;
+   }
 
rc = ibmvnic_wait_for_completion(adapter, >fw_done,
 msecs_to_jiffies(1));
if (rc) {
dev_info(dev,
 "Reset failed, long term map request timed out or 
aborted\n");
+   mutex_unlock(>fw_lock);
return rc;
}
 
@@ -273,8 +284,10 @@ static int reset_long_term_buff(struct ibmvnic_adapter 
*adapter,
dev_info(dev,
 "Reset failed, attempting to free and reallocate 
buffer\n");
free_long_term_buff(adapter, ltb);
+   mutex_unlock(>fw_lock);
return alloc_long_term_buff(adapter, ltb, ltb->size);
}
+   mutex_unlock(>fw_lock);
return 0;
 }
 
@@ -991,19 +1004,26 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter 
*adapter)
if (adapter->vpd->buff)
len = adapter->vpd->len;
 
+   mutex_lock(>fw_lock);
+   adapter->fw_done_rc = 0;
reinit_completion(>fw_done);
+
crq.get_vpd_size.first = IBMVNIC_CRQ_CMD;
crq.get_vpd_size.cmd = GET_VPD_SIZE;
rc = ibmvnic_send_crq(adapter, );
-   if (rc)
+   if (rc) {
+   mutex_unlock(>fw_lock);
return rc;
+   }
 
rc = ibmvnic_wait_for_completion(adapter, >fw_done,
 msecs_to_jiffies(1));
if (rc) {
dev_err(dev, "Could not retrieve VPD size, rc = %d\n", rc);
+   mutex_unlock(>fw_lock);
return rc;
}
+   mutex_unlock(>fw_lock);
 
if (!adapter->vpd->len)
return -ENODATA;
@@ -1030,7 +1050,10 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter 
*adapter)
return -ENOMEM;
}
 
+   mutex_lock(>fw_lock);
+   adapter->fw_done_rc = 0;
reinit_completion(>fw_done);
+
crq.get_vpd.first = IBMVNIC_CRQ_CMD;
crq.get_vpd.cmd = GET_VPD;
crq.get_vpd.ioba = cpu_to_be32(adapter->vpd->dma_addr);
@@ -1039,6 +1062,7 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter 
*adapter)
if (rc) {
kfree(adapter->vpd->buff);
adapter->vpd->buff = NULL;
+   mutex_unlock(>fw_lock);
return rc;
}
 
@@ -1048,9 +1072,11 @@ static int ibmvnic_get_vpd(struct ibmvnic_adapter 
*adapter)
dev_err(dev, "Unable to retrieve VPD, rc = %d\n", rc);

[PATCH net 0/4] ibmvnic: Harden device commands and queries

[Thomas Falcon]

This patch series fixes some shortcomings with the current
VNIC device command implementation. The first patch fixes
the initialization of driver completion structures used
for device commands. Additionally, all waits for device
commands are bounded with a timeout in the event that the
device does not respond or becomes inoperable. Finally,
serialize queries to retain the integrity of device return
codes.

Thomas Falcon (4):
  ibmvnic: Fix completion structure initialization
  ibmvnic: Terminate waiting device threads after loss of service
  ibmvnic: Bound waits for device queries
  ibmvnic: Serialize device queries

 drivers/net/ethernet/ibm/ibmvnic.c | 194 +++--
 drivers/net/ethernet/ibm/ibmvnic.h |   1 +
 2 files changed, 168 insertions(+), 27 deletions(-)

-- 
2.12.3