Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings

2013-05-31 Thread Mike Christie
On 05/28/2013 06:36 AM, Vikas Chaudhary wrote:
 
 
 -Original Message-
 From: Mike Christie micha...@cs.wisc.edu
 Date: Tuesday 28 May 2013 2:19 PM
 To: Vikas vikas.chaudh...@qlogic.com
 Cc: jbottom...@parallels.com jbottom...@parallels.com, scsi
 linux-scsi@vger.kernel.org, Lalit Chandivade
 lalit.chandiv...@qlogic.com, Ravi Anand ravi.an...@qlogic.com, Harish
 Zunjarrao harish.zunjar...@qlogic.com
 Subject: Re: [PATCH 2/8] iscsi_transport: Additional parameters for net
 settings
 
 Come on man, same comments as last patches like this :) Could you use
 the same names that we currently use for existing params?

 Check for the inorder ones and the ones where we use en instead of
 enabled for the postfix. Also check the others.
 
 If we understand correctly you are suggesting to change macro postfix from
 EN to ENABLED as in attached patch iscsi_net_param-fix1.patch.
 But we think changing old macro postfix from ENABLED to EN is better way
 to fix it.

I meant the second one. Also in the patch some of the names of sysfs
files and variables use _enabled, but for sysfs we use _en and then some
names just do not match at all.

For example some iface names like data_pdu_inorder_enabled would be
pdu_inorder_en and the data_seq_inorder_enabled would be
dataseq_inorder_en to match what the naming used in the session. There
are others that need to match like immediate data, etc. Check them
before you resubmit.
--
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings

2013-05-28 Thread Mike Christie
Come on man, same comments as last patches like this :) Could you use
the same names that we currently use for existing params?

Check for the inorder ones and the ones where we use en instead of
enabled for the postfix. Also check the others.

Also what is up with isns. If we support passing the addr/port/enabled
then will it work completely in fw? You do not need any driver or
userspace changes for that?


On 05/09/2013 05:02 AM, vikas.chaudh...@qlogic.com wrote:
 From: Harish Zunjarrao harish.zunjar...@qlogic.com
 
 Added support to display and update additional network parameters
 through iscsiadm
 
 Signed-off-by: Harish Zunjarrao harish.zunjar...@qlogic.com
 Signed-off-by: Vikas Chaudhary vikas.chaudh...@qlogic.com
 ---
  drivers/scsi/scsi_transport_iscsi.c | 331 
 +++-
  include/scsi/iscsi_if.h |  78 +
  include/scsi/scsi_transport_iscsi.h |   3 +
  3 files changed, 411 insertions(+), 1 deletion(-)
 
 diff --git a/drivers/scsi/scsi_transport_iscsi.c 
 b/drivers/scsi/scsi_transport_iscsi.c
 index 133926b..315c8b6 100644
 --- a/drivers/scsi/scsi_transport_iscsi.c
 +++ b/drivers/scsi/scsi_transport_iscsi.c
 @@ -306,11 +306,38 @@ show_##type##_##name(struct device *dev, struct 
 device_attribute *attr, \
   iscsi_iface_attr_show(type, name, ISCSI_NET_PARAM, param)   \
  static ISCSI_IFACE_ATTR(type, name, S_IRUGO, show_##type##_##name, NULL);
  
 -/* generic read only ipvi4 attribute */
 +/* generic read only ipv4 attribute */
  iscsi_iface_net_attr(ipv4_iface, ipaddress, ISCSI_NET_PARAM_IPV4_ADDR);
  iscsi_iface_net_attr(ipv4_iface, gateway, ISCSI_NET_PARAM_IPV4_GW);
  iscsi_iface_net_attr(ipv4_iface, subnet, ISCSI_NET_PARAM_IPV4_SUBNET);
  iscsi_iface_net_attr(ipv4_iface, bootproto, ISCSI_NET_PARAM_IPV4_BOOTPROTO);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_dns_address_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_DNS_ADDR_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_slp_da_info_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_SLP_DA_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_req_isns_info_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_REQ_ISNS_INFO_EN);
 +iscsi_iface_net_attr(ipv4_iface, tos_enabled, ISCSI_NET_PARAM_IPV4_TOS_EN);
 +iscsi_iface_net_attr(ipv4_iface, tos, ISCSI_NET_PARAM_IPV4_TOS);
 +iscsi_iface_net_attr(ipv4_iface, grat_arp_enabled,
 +  ISCSI_NET_PARAM_IPV4_GRAT_ARP_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_alt_client_id_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_ALT_CLIENT_ID_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_alt_client_id,
 +  ISCSI_NET_PARAM_IPV4_DHCP_ALT_CLIENT_ID);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_req_vendor_id_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_REQ_VENDOR_ID_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_use_vendor_id_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_USE_VENDOR_ID_EN);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_vendor_id,
 +  ISCSI_NET_PARAM_IPV4_DHCP_VENDOR_ID);
 +iscsi_iface_net_attr(ipv4_iface, dhcp_learn_iqn_enabled,
 +  ISCSI_NET_PARAM_IPV4_DHCP_LEARN_IQN_EN);
 +iscsi_iface_net_attr(ipv4_iface, fragmentation_enabled,
 +  ISCSI_NET_PARAM_IPV4_FRAGMENT_EN);
 +iscsi_iface_net_attr(ipv4_iface, incoming_forwarding_enabled,
 +  ISCSI_NET_PARAM_IPV4_IN_FORWARD_EN);
 +iscsi_iface_net_attr(ipv4_iface, ttl, ISCSI_NET_PARAM_IPV4_TTL);
  
  /* generic read only ipv6 attribute */
  iscsi_iface_net_attr(ipv6_iface, ipaddress, ISCSI_NET_PARAM_IPV6_ADDR);
 @@ -320,6 +347,27 @@ iscsi_iface_net_attr(ipv6_iface, ipaddr_autocfg,
ISCSI_NET_PARAM_IPV6_ADDR_AUTOCFG);
  iscsi_iface_net_attr(ipv6_iface, link_local_autocfg,
ISCSI_NET_PARAM_IPV6_LINKLOCAL_AUTOCFG);
 +iscsi_iface_net_attr(ipv6_iface, link_local_state,
 +  ISCSI_NET_PARAM_IPV6_LINKLOCAL_STATE);
 +iscsi_iface_net_attr(ipv6_iface, router_state,
 +  ISCSI_NET_PARAM_IPV6_ROUTER_STATE);
 +iscsi_iface_net_attr(ipv6_iface, grat_neighbor_adv_enabled,
 +  ISCSI_NET_PARAM_IPV6_GRAT_NEIGHBOR_ADV_EN);
 +iscsi_iface_net_attr(ipv6_iface, mld_enabled, ISCSI_NET_PARAM_IPV6_MLD_EN);
 +iscsi_iface_net_attr(ipv6_iface, flow_label, 
 ISCSI_NET_PARAM_IPV6_FLOW_LABEL);
 +iscsi_iface_net_attr(ipv6_iface, traffic_class,
 +  ISCSI_NET_PARAM_IPV6_TRAFFIC_CLASS);
 +iscsi_iface_net_attr(ipv6_iface, hop_limit, ISCSI_NET_PARAM_IPV6_HOP_LIMIT);
 +iscsi_iface_net_attr(ipv6_iface, nd_reachable_tmo,
 +  ISCSI_NET_PARAM_IPV6_ND_REACHABLE_TMO);
 +iscsi_iface_net_attr(ipv6_iface, nd_rexmit_time,
 +  ISCSI_NET_PARAM_IPV6_ND_REXMIT_TIME);
 +iscsi_iface_net_attr(ipv6_iface, nd_stale_tmo,
 +  ISCSI_NET_PARAM_IPV6_ND_STALE_TMO);
 +iscsi_iface_net_attr(ipv6_iface, dup_addr_det_cnt,
 +  ISCSI_NET_PARAM_IPV6_DUP_ADDR_DET_CNT);
 +iscsi_iface_net_attr(ipv6_iface, 

Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings

2013-05-28 Thread Mike Christie
On 05/28/2013 03:49 AM, Mike Christie wrote:
 Come on man, same comments as last patches like this :) Could you use
 the same names that we currently use for existing params?
 
 Check for the inorder ones and the ones where we use en instead of
 enabled for the postfix. Also check the others.
 
 Also what is up with isns. If we support passing the addr/port/enabled
 then will it work completely in fw? You do not need any driver or
 userspace changes for that?

For isns support does what you are adding work for flash mode only?

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


Re: [PATCH 2/8] iscsi_transport: Additional parameters for net settings

2013-05-28 Thread Vikas Chaudhary


-Original Message-
From: Mike Christie micha...@cs.wisc.edu
Date: Tuesday 28 May 2013 2:19 PM
To: Vikas vikas.chaudh...@qlogic.com
Cc: jbottom...@parallels.com jbottom...@parallels.com, scsi
linux-scsi@vger.kernel.org, Lalit Chandivade
lalit.chandiv...@qlogic.com, Ravi Anand ravi.an...@qlogic.com, Harish
Zunjarrao harish.zunjar...@qlogic.com
Subject: Re: [PATCH 2/8] iscsi_transport: Additional parameters for net
settings

Come on man, same comments as last patches like this :) Could you use
the same names that we currently use for existing params?

Check for the inorder ones and the ones where we use en instead of
enabled for the postfix. Also check the others.

If we understand correctly you are suggesting to change macro postfix from
EN to ENABLED as in attached patch iscsi_net_param-fix1.patch.
But we think changing old macro postfix from ENABLED to EN is better way
to fix it. It will make iscsi_if.h consistent for iscsi_net_param,
iscsi_param and iscsi_flashnode_param macros as defined in attached patch
iscsi_net_param-fix2.patch.

Let us know what you think?



Also what is up with isns. If we support passing the addr/port/enabled
then will it work completely in fw? You do not need any driver or
userspace changes for that?

For now we are just adding support to enable or disable iSNS.
However we need to modify driver and userspace to support iSNS.

attachment: winmail.dat