diff mbox

[PATCHv2] iscsi_ibft: Add prefix-len attr and display netmask

Message ID 21ae62f67edf39ab09f6c5aa842b89fd7d6e1dc6.1456423714.git.lduncan@suse.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Lee Duncan Feb. 25, 2016, 6:15 p.m. UTC 
From: Hannes Reinecke <hare@suse.de>

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.

Some older user-space code might rely on the netmask attribute
being present, so we should always display it.

Changes from v1:
 - Combined two patches into one

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Lee Duncan <lduncan@suse.com>
---
 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(+)

Comments

Mike Christie Feb. 29, 2016, 6:45 p.m. UTC | #1 
On 02/25/2016 12:15 PM, Lee Duncan wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> 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.

The code looks ok to me, but I think this ipv4 comment was left over
from the last posting since it conflicts with the comment below about
always displaying it. Maybe it can just be edited when the patch is
merged to avoid having to resend again. If so,

Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>


> 
> Some older user-space code might rely on the netmask attribute
> being present, so we should always display it.
> 
> Changes from v1:
>  - Combined two patches into one
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: Lee Duncan <lduncan@suse.com>
> ---
>  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(+)
> 
> diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
> index 72791232e46b..81037e5fe301 100644
> --- a/drivers/firmware/iscsi_ibft.c
> +++ b/drivers/firmware/iscsi_ibft.c
> @@ -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,6 +463,7 @@ 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_PREFIX_LEN:
>  	case ISCSI_BOOT_ETH_SUBNET_MASK:
>  		if (nic->subnet_mask_prefix)
>  			rc = S_IRUGO;
> diff --git a/drivers/scsi/iscsi_boot_sysfs.c b/drivers/scsi/iscsi_boot_sysfs.c
> index 680bf6f0ce76..8f0ea97cf31f 100644
> --- a/drivers/scsi/iscsi_boot_sysfs.c
> +++ b/drivers/scsi/iscsi_boot_sysfs.c
> @@ -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);
> diff --git a/include/linux/iscsi_boot_sysfs.h b/include/linux/iscsi_boot_sysfs.h
> index 2a8b1659bf35..548d55395488 100644
> --- a/include/linux/iscsi_boot_sysfs.h
> +++ b/include/linux/iscsi_boot_sysfs.h
> @@ -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,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Konrad Rzeszutek Wilk Feb. 29, 2016, 9:04 p.m. UTC | #2 
On Mon, Feb 29, 2016 at 1:45 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
> On 02/25/2016 12:15 PM, Lee Duncan wrote:
>> From: Hannes Reinecke <hare@suse.de>
>>
>> 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.
>
> The code looks ok to me, but I think this ipv4 comment was left over
> from the last posting since it conflicts with the comment below about
> always displaying it. Maybe it can just be edited when the patch is
> merged to avoid having to resend again. If so,
>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
>

Cool. Let me roll it up and send the git pull to Linus.

This does not seem be super-urgent so it can wait for the upcoming merge window?

>
>>
>> Some older user-space code might rely on the netmask attribute
>> being present, so we should always display it.
>>
>> Changes from v1:
>>  - Combined two patches into one
>>
>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>> Signed-off-by: Lee Duncan <lduncan@suse.com>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lee Duncan Feb. 29, 2016, 9:25 p.m. UTC | #3 
On 02/29/2016 10:45 AM, Mike Christie wrote:
> On 02/25/2016 12:15 PM, Lee Duncan wrote:
>> From: Hannes Reinecke <hare@suse.de>
>>
>> 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.
> 
> The code looks ok to me, but I think this ipv4 comment was left over
> from the last posting since it conflicts with the comment below about
> always displaying it. Maybe it can just be edited when the patch is
> merged to avoid having to resend again. If so,
> 
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
> 
> ...

Since there are no other comments, I will resubmit it with the comment
fixed.
Lee Duncan Feb. 29, 2016, 9:49 p.m. UTC | #4 
On 02/29/2016 01:04 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Feb 29, 2016 at 1:45 PM, Mike Christie <michaelc@cs.wisc.edu> wrote:
>> On 02/25/2016 12:15 PM, Lee Duncan wrote:
>>> From: Hannes Reinecke <hare@suse.de>
>>>
>>> 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.
>>
>> The code looks ok to me, but I think this ipv4 comment was left over
>> from the last posting since it conflicts with the comment below about
>> always displaying it. Maybe it can just be edited when the patch is
>> merged to avoid having to resend again. If so,
>>
>> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
>>
> 
> Cool. Let me roll it up and send the git pull to Linus.

I actually just resent it, but the only change was to truncate the last
line of first paragraph of the comment to:

    So introduce a new attribute 'prefix-len'.


> 
> This does not seem be super-urgent so it can wait for the upcoming merge window?
> 
>>
>>>
>>> Some older user-space code might rely on the netmask attribute
>>> being present, so we should always display it.
>>>
>>> Changes from v1:
>>>  - Combined two patches into one
>>>
>>> Signed-off-by: Hannes Reinecke <hare@suse.de>
>>> Signed-off-by: Lee Duncan <lduncan@suse.com>
>
diff mbox

Patch

diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
index 72791232e46b..81037e5fe301 100644
--- a/drivers/firmware/iscsi_ibft.c
+++ b/drivers/firmware/iscsi_ibft.c
@@ -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,6 +463,7 @@  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_PREFIX_LEN:
 	case ISCSI_BOOT_ETH_SUBNET_MASK:
 		if (nic->subnet_mask_prefix)
 			rc = S_IRUGO;
diff --git a/drivers/scsi/iscsi_boot_sysfs.c b/drivers/scsi/iscsi_boot_sysfs.c
index 680bf6f0ce76..8f0ea97cf31f 100644
--- a/drivers/scsi/iscsi_boot_sysfs.c
+++ b/drivers/scsi/iscsi_boot_sysfs.c
@@ -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);
diff --git a/include/linux/iscsi_boot_sysfs.h b/include/linux/iscsi_boot_sysfs.h
index 2a8b1659bf35..548d55395488 100644
--- a/include/linux/iscsi_boot_sysfs.h
+++ b/include/linux/iscsi_boot_sysfs.h
@@ -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,