Re: [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI

2008-08-23 Thread Andrew Morton

On Fri, 22 Aug 2008 11:38:32 -0700
Karen Xie [EMAIL PROTECTED] wrote:

 [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI
 
 From: Karen Xie [EMAIL PROTECTED]
 
 Create a per port sysfs entry to pass an IP address to the NIC driver, and a 
 control call for the iSCSI driver to grab it.
 The IP address is required in both drivers to manage ARP requests and 
 connection set up.
 
 Signed-off-by: Divy Le Ray [EMAIL PROTECTED]
 ---
 
  drivers/net/cxgb3/adapter.h|1 +
  drivers/net/cxgb3/cxgb3_ctl_defs.h |7 
  drivers/net/cxgb3/cxgb3_main.c |   65 
 
  drivers/net/cxgb3/cxgb3_offload.c  |6 +++
  4 files changed, 79 insertions(+), 0 deletions(-)
 
 diff --git a/drivers/net/cxgb3/adapter.h b/drivers/net/cxgb3/adapter.h
 index 2711404..0e4fe95 100644
 --- a/drivers/net/cxgb3/adapter.h
 +++ b/drivers/net/cxgb3/adapter.h
 @@ -64,6 +64,7 @@ struct port_info {
   struct link_config link_config;
   struct net_device_stats netstats;
   int activity;
 + __be32 iscsi_ipaddr;
  };
  
  enum {   /* adapter flags */
 diff --git a/drivers/net/cxgb3/cxgb3_ctl_defs.h 
 b/drivers/net/cxgb3/cxgb3_ctl_defs.h
 index 6ad9240..e171aa8 100644
 --- a/drivers/net/cxgb3/cxgb3_ctl_defs.h
 +++ b/drivers/net/cxgb3/cxgb3_ctl_defs.h
 @@ -57,6 +57,7 @@ enum {
   RDMA_GET_MIB= 19,
  
   GET_RX_PAGE_INFO= 50,
 + GET_ISCSI_IPADDR= 51,
  };
  
  /*
 @@ -86,6 +87,12 @@ struct iff_mac {
   u16 vlan_tag;
  };
  
 +/* Structure used to request a port's iSCSI IP address */
 +struct iscsi_ipaddr {
 + struct net_device *dev; /* the net_device */
 + __be32 ipaddr;  /* the return iSCSI IP address */
 +};
 +
  struct pci_dev;
  
  /*
 diff --git a/drivers/net/cxgb3/cxgb3_main.c b/drivers/net/cxgb3/cxgb3_main.c
 index 5447f3e..1c8952c 100644
 --- a/drivers/net/cxgb3/cxgb3_main.c
 +++ b/drivers/net/cxgb3/cxgb3_main.c
 @@ -687,6 +687,66 @@ static struct attribute *offload_attrs[] = {
  
  static struct attribute_group offload_attr_group = {.attrs = offload_attrs };
  
 +static ssize_t iscsi_ipaddr_attr_show(struct device *d,
 +   char *buf)

could fit in a single 80-col line.

 +{
 + struct port_info *pi = netdev_priv(to_net_dev(d));
 + __be32 a = ntohl(pi-iscsi_ipaddr);
 +
 + return sprintf(buf, %d.%d.%d.%d\n,
 +(a  24)  0xff,
 +(a  16)  0xff,
 +(a   8)  0xff,
 +(a   0)  0xff);

Use NIPQUAD and NIPQUAD_FMT here?

 +}
 +
 +static ssize_t iscsi_ipaddr_attr_store(struct device *d,
 +const char *buf, size_t len)
 +{
 + struct port_info *pi = netdev_priv(to_net_dev(d));
 + __be32 a = 0;

There's not really any need to use __be32 in kernel code.  Plain old
be23 is fine.

 + unsigned long octet;
 + const char *parse = buf;
 + char *endp;
 + int i;
 +
 + for (i = 1; i = 4; i++) {
 + octet = simple_strtoul(parse, endp, 10);
 + if (endp == buf || octet  255 ||
 + (i  4  *endp != '.') ||
 + (i == 4  *endp != '\0'  *endp != '\n'))
 + return -EINVAL;
 + a = (a  8) | octet;
 + parse = endp+1;
 + }
 + pi-iscsi_ipaddr = htonl(a);
 + return endp-buf;
 +}

This appears to be taking a dotted quad ipv4 address in ascii form,
turning it into a u32 while performing checking?

Surely we have a library function somewhere in networking which does
this?  If not, I'd suggest writing one. 

 +#define ISCSI_IPADDR_ATTR(name) \
 +static ssize_t show_##name(struct device *d, struct device_attribute *attr, \
 +char *buf) \
 +{ \
 + return iscsi_ipaddr_attr_show(d, buf); \
 +} \
 +static ssize_t store_##name(struct device *d, struct device_attribute *attr, 
 \
 +const char *buf, size_t len) \
 +{ \
 + return iscsi_ipaddr_attr_store(d, buf, len); \
 +} \
 +static DEVICE_ATTR(name, S_IRUGO | S_IWUSR, show_##name, store_##name)
 +
 +ISCSI_IPADDR_ATTR(iscsi_ipaddr);
 +
 +static struct attribute *iscsi_offload_attrs[] = {
 + dev_attr_iscsi_ipaddr.attr,
 + NULL
 +};
 +
 +static struct attribute_group iscsi_offload_attr_group = {
 +.attrs = iscsi_offload_attrs
 +};
 +
  /*
   * Sends an sk_buff to an offload queue driver
   * after dealing with any active network taps.
 @@ -1078,6 +1138,7 @@ static int cxgb_open(struct net_device *dev)
   if (err)
   printk(KERN_WARNING
  Could not initialize offload capabilities\n);
 + sysfs_create_group(dev-dev.kobj, iscsi_offload_attr_group);
   }
  
   link_start(dev);
 @@ -1100,6 +1161,9 @@ static int cxgb_close(struct net_device *dev)
   netif_carrier_off(dev);
   t3_mac_disable(pi-mac, MAC_DIRECTION_TX

Re: [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI

2008-08-23 Thread Andrew Morton

On Fri, 22 Aug 2008 14:17:18 -0500
Steve Wise [EMAIL PROTECTED] wrote:

 Andrew Morton wrote:
  +  unsigned long octet;
  +  const char *parse = buf;
  +  char *endp;
  +  int i;
  +
  +  for (i = 1; i = 4; i++) {
  +  octet = simple_strtoul(parse, endp, 10);
  +  if (endp == buf || octet  255 ||
  +  (i  4  *endp != '.') ||
  +  (i == 4  *endp != '\0'  *endp != '\n'))
  +  return -EINVAL;
  +  a = (a  8) | octet;
  +  parse = endp+1;
  +  }
  +  pi-iscsi_ipaddr = htonl(a);
  +  return endp-buf;
  +}
  
 
  This appears to be taking a dotted quad ipv4 address in ascii form,
  turning it into a u32 while performing checking?
 
  Surely we have a library function somewhere in networking which does
  this?  If not, I'd suggest writing one. 
 

 
 try in_aton() from include/linux/inet.h.
 

yeah.  But that function is a crock.  No error checking at all!

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI

2008-08-23 Thread Steve Wise

Andrew Morton wrote:
 +unsigned long octet;
 +const char *parse = buf;
 +char *endp;
 +int i;
 +
 +for (i = 1; i = 4; i++) {
 +octet = simple_strtoul(parse, endp, 10);
 +if (endp == buf || octet  255 ||
 +(i  4  *endp != '.') ||
 +(i == 4  *endp != '\0'  *endp != '\n'))
 +return -EINVAL;
 +a = (a  8) | octet;
 +parse = endp+1;
 +}
 +pi-iscsi_ipaddr = htonl(a);
 +return endp-buf;
 +}
 

 This appears to be taking a dotted quad ipv4 address in ascii form,
 turning it into a u32 while performing checking?

 Surely we have a library function somewhere in networking which does
 this?  If not, I'd suggest writing one. 

   

try in_aton() from include/linux/inet.h.





--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI

2008-08-23 Thread Steve Wise

Andrew Morton wrote:
 On Fri, 22 Aug 2008 14:17:18 -0500
 Steve Wise [EMAIL PROTECTED] wrote:

   
 Andrew Morton wrote:
 
 +  unsigned long octet;
 +  const char *parse = buf;
 +  char *endp;
 +  int i;
 +
 +  for (i = 1; i = 4; i++) {
 +  octet = simple_strtoul(parse, endp, 10);
 +  if (endp == buf || octet  255 ||
 +  (i  4  *endp != '.') ||
 +  (i == 4  *endp != '\0'  *endp != '\n'))
 +  return -EINVAL;
 +  a = (a  8) | octet;
 +  parse = endp+1;
 +  }
 +  pi-iscsi_ipaddr = htonl(a);
 +  return endp-buf;
 +}
 
 
 This appears to be taking a dotted quad ipv4 address in ascii form,
 turning it into a u32 while performing checking?

 Surely we have a library function somewhere in networking which does
 this?  If not, I'd suggest writing one. 

   
   
 try in_aton() from include/linux/inet.h.

 

 yeah.  But that function is a crock.  No error checking at all!
   

Oh you want error checking?

:)

Yea if this is a user/sysadmin supplied value, then we need a rubust 
inet_aton() in the kernel to validate it...



--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: [PATCH 1/4 2.6.28] cxgb3 - manage a private ip address for iSCSI

2008-08-23 Thread Herbert Xu

On Fri, Aug 22, 2008 at 07:08:51PM +, Andrew Morton wrote:

 +static ssize_t iscsi_ipaddr_attr_store(struct device *d,
 +const char *buf, size_t len)
 +{
 + struct port_info *pi = netdev_priv(to_net_dev(d));
 + __be32 a = 0;
 
 There's not really any need to use __be32 in kernel code.  Plain old
 be23 is fine.

Sorry but where is be32 defined? I couldn't find it in linux/types.h.

Perhaps we should resurrect this patch from 2005?

[PATCH] Add be*/le* types without underscores

I've seen a number of patches that have started to use the __le*/__be*
types within the kernel.  Nice as they are, the underscores are really
a bit of an eye sore.  Since there seems to be no name conflict within
the kernel, why don't we use them without the underscores like just as
we do with types like u32?

Here is a patch to do just that.  I've verified that there are no
conflicts by grepping the current git tree and then building it with
the patch.

Of course userspace won't see them since they're protected by
#ifdef __KERNEL__.

Signed-off-by: Herbert Xu [EMAIL PROTECTED]

diff --git a/fs/ntfs/types.h b/fs/ntfs/types.h
index 8c8053b..79b44c7 100644
--- a/fs/ntfs/types.h
+++ b/fs/ntfs/types.h
@@ -25,9 +25,6 @@
 
 #include linux/types.h
 
-typedef __le16 le16;
-typedef __le32 le32;
-typedef __le64 le64;
 typedef __u16 __bitwise sle16;
 typedef __u32 __bitwise sle32;
 typedef __u64 __bitwise sle64;
diff --git a/include/linux/types.h b/include/linux/types.h
index d4a9ce6..61747dc 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -189,6 +189,15 @@ typedef __u16 __bitwise __sum16;
 typedef __u32 __bitwise __wsum;
 
 #ifdef __KERNEL__
+typedef __le16 le16;
+typedef __be16 be16;
+typedef __le32 le32;
+typedef __be32 be32;
+#if defined(__GNUC__)
+typedef __le64 le64;
+typedef __be64 be64;
+#endif
+
 typedef unsigned __bitwise__ gfp_t;
 
 #ifdef CONFIG_RESOURCES_64BIT

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmVHI~} [EMAIL PROTECTED]
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
open-iscsi group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---