RESEND: Enable iBFT IPv6 booting with prefix length

PATCH Superseded Lee Duncan 3 participants 6 messages Jan 22, 2016 – Feb 25, 2016

Subsystems (3)

SCSI iSCSI iSCSI boot firmware table (iBFT) driver

lore

Share & tools

lore

Lee Duncan

author maintainer

2016-01-22 19:51 UTC #0 lore raw

cover RESEND: Enable iBFT IPv6 booting with prefix length

[Resending. Apologies if you get this twice.]

It turns out that IPv6 doesn't care about netmask, but instead uses "prefix length" to determine the subnet, but the iBFT subsystem still just supports netmask.

These two patches enable IPv6 iBFT (iscsi booting) by adding support for IPv6 prefix and prefix length. Open-iscsi changes to use these new values will be introduced once these changes are present.

Hannes Reinecke (2):
iscsi_ibft: Add prefix-len attribute
iscsi_ibft: Always display netmask

 drivers/firmware/iscsi_ibft.c    | 4 ++++
 drivers/scsi/iscsi_boot_sysfs.c  | 5 +++++
 include/linux/iscsi_boot_sysfs.h | 1 +
 3 files changed, 10 insertions(+)

Signature · 1 line

1.8.5.2

Lee Duncan

author maintainer

2016-01-22 19:51 UTC #1 lore raw

PATCH 1/2 iscsi_ibft: Add prefix-len attribute

From: Hannes Reinecke <[email protected]>

The iBFT table only specifies a prefix length, not a netmask. And the netmask is pretty much pointless for IPv6. So introduce a new attribute 'prefix-len' and display the netmask attribute only for IPv4 addresses.

Signed-off-by Hannes Reinecke <[email protected]>

Signed-off-by Lee Duncan <[email protected]>

---
 drivers/firmware/iscsi_ibft.c    | 12 +++++++++++-
 drivers/scsi/iscsi_boot_sysfs.c  |  5 +++++
 include/linux/iscsi_boot_sysfs.h |  1 +
 3 files changed, 17 insertions(+), 1 deletion(-)

drivers/firmware/iscsi_ibft.c +11 -1

index 72791232e46b..2dd1fbb8cccc 100644
@@ -319,6 +319,9 @@ static ssize_t ibft_attr_show_nic(void *data, int type, char *buf)
 		val = cpu_to_be32(~((1 << (32-nic->subnet_mask_prefix))-1));
 		str += sprintf(str, "%pI4", &val);
 		break;
+	case ISCSI_BOOT_ETH_PREFIX_LEN:
+		str += sprintf(str, "%d\n", nic->subnet_mask_prefix);
+		break;
 	case ISCSI_BOOT_ETH_ORIGIN:
 		str += sprintf(str, "%d\n", nic->origin);
 		break;

@@ -460,10 +463,17 @@ static umode_t ibft_check_nic_for(void *data, int type)
 		if (address_not_null(nic->ip_addr))
 			rc = S_IRUGO;
 		break;
-	case ISCSI_BOOT_ETH_SUBNET_MASK:
+	case ISCSI_BOOT_ETH_PREFIX_LEN:
 		if (nic->subnet_mask_prefix)
 			rc = S_IRUGO;
 		break;
+	case ISCSI_BOOT_ETH_SUBNET_MASK:
+		if (!memcmp(nic->ip_addr, nulls, 10) &&
+		    (nic->ip_addr[10] == 0xff) &&
+		    (nic->ip_addr[11] == 0xff) &&
+		    nic->subnet_mask_prefix)
+			rc = S_IRUGO;
+		break;
 	case ISCSI_BOOT_ETH_ORIGIN:
 		rc = S_IRUGO;
 		break;

drivers/scsi/iscsi_boot_sysfs.c +5 -0

index 680bf6f0ce76..8f0ea97cf31f 100644
@@ -166,6 +166,7 @@ static struct attribute_group iscsi_boot_target_attr_group = {
 iscsi_boot_rd_attr(eth_index, index, ISCSI_BOOT_ETH_INDEX);
 iscsi_boot_rd_attr(eth_flags, flags, ISCSI_BOOT_ETH_FLAGS);
 iscsi_boot_rd_attr(eth_ip, ip-addr, ISCSI_BOOT_ETH_IP_ADDR);
+iscsi_boot_rd_attr(eth_prefix, prefix-len, ISCSI_BOOT_ETH_PREFIX_LEN);
 iscsi_boot_rd_attr(eth_subnet, subnet-mask, ISCSI_BOOT_ETH_SUBNET_MASK);
 iscsi_boot_rd_attr(eth_origin, origin, ISCSI_BOOT_ETH_ORIGIN);
 iscsi_boot_rd_attr(eth_gateway, gateway, ISCSI_BOOT_ETH_GATEWAY);

@@ -181,6 +182,7 @@ static struct attribute *ethernet_attrs[] = {
 	&iscsi_boot_attr_eth_index.attr,
 	&iscsi_boot_attr_eth_flags.attr,
 	&iscsi_boot_attr_eth_ip.attr,
+	&iscsi_boot_attr_eth_prefix.attr,
 	&iscsi_boot_attr_eth_subnet.attr,
 	&iscsi_boot_attr_eth_origin.attr,
 	&iscsi_boot_attr_eth_gateway.attr,

@@ -208,6 +210,9 @@ static umode_t iscsi_boot_eth_attr_is_visible(struct kobject *kobj,
 	else if (attr ==  &iscsi_boot_attr_eth_ip.attr)
 		return boot_kobj->is_visible(boot_kobj->data,
 					     ISCSI_BOOT_ETH_IP_ADDR);
+	else if (attr ==  &iscsi_boot_attr_eth_prefix.attr)
+		return boot_kobj->is_visible(boot_kobj->data,
+					     ISCSI_BOOT_ETH_PREFIX_LEN);
 	else if (attr ==  &iscsi_boot_attr_eth_subnet.attr)
 		return boot_kobj->is_visible(boot_kobj->data,
 					     ISCSI_BOOT_ETH_SUBNET_MASK);

include/linux/iscsi_boot_sysfs.h +1 -0

index 2a8b1659bf35..548d55395488 100644
@@ -23,6 +23,7 @@ enum iscsi_boot_eth_properties_enum {
 	ISCSI_BOOT_ETH_INDEX,
 	ISCSI_BOOT_ETH_FLAGS,
 	ISCSI_BOOT_ETH_IP_ADDR,
+	ISCSI_BOOT_ETH_PREFIX_LEN,
 	ISCSI_BOOT_ETH_SUBNET_MASK,
 	ISCSI_BOOT_ETH_ORIGIN,
 	ISCSI_BOOT_ETH_GATEWAY,

Signature · 1 line

1.8.5.2

Lee Duncan

author maintainer

2016-01-22 19:51 UTC #2 lore raw

PATCH 2/2 iscsi_ibft: Always display netmask

In reply to Lee Duncan

From: Hannes Reinecke <[email protected]>

Some older user-space code might rely on the netmask attribute being present, so we should always display it.
This fixes a regression introduced by commit 0b2eb3c4060a16f3ec11a4d6d4c934e7e5d5334f.

Signed-off-by Hannes Reinecke <[email protected]>

Signed-off-by Lee Duncan <[email protected]>

---
 drivers/firmware/iscsi_ibft.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

drivers/firmware/iscsi_ibft.c +1 -7

index 2dd1fbb8cccc..81037e5fe301 100644
@@ -464,14 +464,8 @@ static umode_t ibft_check_nic_for(void *data, int type)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_PREFIX_LEN:
-		if (nic->subnet_mask_prefix)
-			rc = S_IRUGO;
-		break;
 	case ISCSI_BOOT_ETH_SUBNET_MASK:
-		if (!memcmp(nic->ip_addr, nulls, 10) &&
-		    (nic->ip_addr[10] == 0xff) &&
-		    (nic->ip_addr[11] == 0xff) &&
-		    nic->subnet_mask_prefix)
+		if (nic->subnet_mask_prefix)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_ORIGIN:

Signature · 1 line

1.8.5.2

Mike Christie

2016-02-02 06:45 UTC #3 lore raw

Re: [PATCH 2/2] iscsi_ibft: Always display netmask

Quoted 24 lines · Lee Duncan

From: Hannes Reinecke <[email protected]>
Some older user-space code might rely on the netmask attribute being present, so we should always display it.
This fixes a regression introduced by commit 0b2eb3c4060a16f3ec11a4d6d4c934e7e5d5334f.
Signed-off-by Hannes Reinecke <[email protected]>
Signed-off-by Lee Duncan <[email protected]>
---
 drivers/firmware/iscsi_ibft.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/firmware/iscsi_ibft.c +0 -2
index 2dd1fbb8cccc..81037e5fe301 100644
@@ -464,14 +464,8 @@ static umode_t ibft_check_nic_for(void *data, int type)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_PREFIX_LEN:
-		if (nic->subnet_mask_prefix)
-			rc = S_IRUGO;

drivers/firmware/iscsi_ibft.c static umode_t ibft_check_nic_for(void *data, int type) · line 471

Quoted 10 lines

drivers/firmware/iscsi_ibft.c +1 -5

-		break;
 	case ISCSI_BOOT_ETH_SUBNET_MASK:
-		if (!memcmp(nic->ip_addr, nulls, 10) &&
-		    (nic->ip_addr[10] == 0xff) &&
-		    (nic->ip_addr[11] == 0xff) &&
-		    nic->subnet_mask_prefix)
+		if (nic->subnet_mask_prefix)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_ORIGIN:

Sorry. I thought I sent this mail already.

Is the commit id above supposed to be referencing the first patch? I could not match it to anything. If so, then shouldn't this patch just be combined with the second patch and some comment about us always displaying it for compat reasons added to the code?

Also, you should normally cc Konrad for iscsi_ibft.c patches, because he is actually the maintainer.

Lee Duncan

author maintainer

2016-02-25 16:52 UTC #4 lore raw

Re: [PATCH 2/2] iscsi_ibft: Always display netmask

Quoted 38 lines · Mike Christie

Quoted 24 lines · Lee Duncan
From: Hannes Reinecke <[email protected]>
Some older user-space code might rely on the netmask attribute being present, so we should always display it.
This fixes a regression introduced by commit 0b2eb3c4060a16f3ec11a4d6d4c934e7e5d5334f.
Signed-off-by Hannes Reinecke <[email protected]>
Signed-off-by Lee Duncan <[email protected]>
---
 drivers/firmware/iscsi_ibft.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
drivers/firmware/iscsi_ibft.c +0 -2
index 2dd1fbb8cccc..81037e5fe301 100644
@@ -464,14 +464,8 @@ static umode_t ibft_check_nic_for(void *data, int type)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_PREFIX_LEN:
-		if (nic->subnet_mask_prefix)
-			rc = S_IRUGO;
drivers/firmware/iscsi_ibft.c static umode_t ibft_check_nic_for(void *data, int type) · line 471
Quoted 10 lines
drivers/firmware/iscsi_ibft.c +1 -5
-		break;
 	case ISCSI_BOOT_ETH_SUBNET_MASK:
-		if (!memcmp(nic->ip_addr, nulls, 10) &&
-		    (nic->ip_addr[10] == 0xff) &&
-		    (nic->ip_addr[11] == 0xff) &&
-		    nic->subnet_mask_prefix)
+		if (nic->subnet_mask_prefix)
 			rc = S_IRUGO;
 		break;
 	case ISCSI_BOOT_ETH_ORIGIN:
Sorry. I thought I sent this mail already.

Quoted 7 lines

Is the commit id above supposed to be referencing the first patch? I could not match it to anything. If so, then shouldn't this patch just be combined with the second patch and some comment about us always displaying it for compat reasons added to the code?
Also, you should normally cc Konrad for iscsi_ibft.c patches, because he is actually the maintainer.

Hi Mike:

I'm sorry I didn't reply sooner. I let this get buried in a side folder and missed it.

The commit ID in Patch 2 was from the SUSE repository. The bottom line is that I think you are correct, these two patches could easily be combined. I will resubmit them as one combined patch.

I submitted them as two because I was just feeding patching that Hannes had done upstream, but I should have noticed they could be combined.

Signature · 2 lines

Lee Duncan
SUSE Labs

Martin K. Petersen

2016-02-02 01:17 UTC #5 lore raw

In reply to Lee Duncan

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Lee" == Lee Duncan <[email protected]> writes:

Lee> [Resending. Apologies if you get this twice.] It turns out that Lee> IPv6 doesn't care about netmask, but instead uses "prefix length" Lee> to determine the subnet, but the iBFT subsystem still just supports Lee> netmask.

Mike, care to review?

	https://patchwork.kernel.org/patch/8092511/
	https://patchwork.kernel.org/patch/8092501/

Signature · 1 line

Martin K. Petersen	Oracle Linux Engineering