Netdev Archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v4 1/4] net core: Add protodown support.
@ 2015-07-08 21:04 anuradhak
  2015-07-08 23:07 ` Jonathan Toppins
  2015-07-15 17:31 ` Stephen Hemminger
  0 siblings, 2 replies; 5+ messages in thread
From: anuradhak @ 2015-07-08 21:04 UTC (permalink / raw
  To: davem, sfeldma; +Cc: netdev, roopa, gospo, wkok, anuradhak

From: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>

This patch introduces the IF_PROTOF_DOWN flag via proto_flags that can be
used by user space applications to notify switch drivers that errors have
been detected on the device.

The switch driver can react to protodown notification by doing a phys down
on the associated switch port.

Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
Signed-off-by: Andy Gospodarek <gospo@cumulusnetworks.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Wilson Kok <wkok@cumulusnetworks.com>
---
 include/linux/netdevice.h |   13 +++++++++++++
 include/uapi/linux/if.h   |    6 ++++++
 net/core/dev.c            |   20 ++++++++++++++++++++
 net/core/net-sysfs.c      |   14 ++++++++++++++
 4 files changed, 53 insertions(+)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e20979d..99ebb01 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1041,6 +1041,11 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
  *	TX queue.
  * int (*ndo_get_iflink)(const struct net_device *dev);
  *	Called to get the iflink value of this device.
+ * void (*ndo_change_proto_flags)(struct net_device *dev,
+ *				  unsigned int proto_flags);
+ *	This function is used to pass protocol port state information
+ *	to the switch driver.
+ *
  */
 struct net_device_ops {
 	int			(*ndo_init)(struct net_device *dev);
@@ -1211,6 +1216,8 @@ struct net_device_ops {
 						      int queue_index,
 						      u32 maxrate);
 	int			(*ndo_get_iflink)(const struct net_device *dev);
+	int			(*ndo_change_proto_flags)(struct net_device *dev,
+							  unsigned int proto_flags);
 };
 
 /**
@@ -1502,6 +1509,10 @@ enum netdev_priv_flags {
  *
  *	@qdisc_tx_busylock:	XXX: need comments on this one
  *
+ *	@proto_flags:	protocol port state information can be sent to the
+ *			switch driver and used to set the phys state of the
+ *			switch port.
+ *
  *	FIXME: cleanup struct net_device such that network protocol info
  *	moves out.
  */
@@ -1762,6 +1773,7 @@ struct net_device {
 #endif
 	struct phy_device *phydev;
 	struct lock_class_key *qdisc_tx_busylock;
+	unsigned int proto_flags;
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 
@@ -2982,6 +2994,7 @@ int dev_get_phys_port_id(struct net_device *dev,
 			 struct netdev_phys_item_id *ppid);
 int dev_get_phys_port_name(struct net_device *dev,
 			   char *name, size_t len);
+int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags);
 struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
 struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				    struct netdev_queue *txq, int *ret);
diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
index 9cf2394..8d60fe7 100644
--- a/include/uapi/linux/if.h
+++ b/include/uapi/linux/if.h
@@ -156,6 +156,12 @@ enum {
 	IF_LINK_MODE_DORMANT,	/* limit upward transition to dormant */
 };
 
+/* proto_flags - port state information can be passed to the switch driver and
+ * used to determine the phys state of the switch port */
+enum {
+	IF_PROTOF_DOWN		= 1<<0	/* set switch port phys state down */
+};
+
 /*
  *	Device mapping structure. I'd just gone off and designed a 
  *	beautiful scheme using only loadable modules with arguments
diff --git a/net/core/dev.c b/net/core/dev.c
index 6778a99..87571c4 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6076,6 +6076,26 @@ int dev_get_phys_port_name(struct net_device *dev,
 EXPORT_SYMBOL(dev_get_phys_port_name);
 
 /**
+ *	dev_change_proto_flags - update protocol port state information
+ *	@dev: device
+ *	@proto_flags: new value
+ *
+ *	This info can be used by switch drivers to set the phys state of the
+ *	port.
+ */
+int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags)
+{
+	const struct net_device_ops *ops = dev->netdev_ops;
+
+	if (dev->proto_flags == proto_flags)
+		return 0;
+	if (!ops->ndo_change_proto_flags)
+		return -EOPNOTSUPP;
+	return ops->ndo_change_proto_flags(dev, proto_flags);
+}
+EXPORT_SYMBOL(dev_change_proto_flags);
+
+/**
  *	dev_new_index	-	allocate an ifindex
  *	@net: the applicable net namespace
  *
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 18b34d7..06f355e 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -404,6 +404,19 @@ static ssize_t group_store(struct device *dev, struct device_attribute *attr,
 NETDEVICE_SHOW(group, fmt_dec);
 static DEVICE_ATTR(netdev_group, S_IRUGO | S_IWUSR, group_show, group_store);
 
+static int change_proto_flags(struct net_device *dev, unsigned long proto_flags)
+{
+	return dev_change_proto_flags(dev, (unsigned int)proto_flags);
+}
+
+static ssize_t proto_flags_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t len)
+{
+	return netdev_store(dev, attr, buf, len, change_proto_flags);
+}
+NETDEVICE_SHOW_RW(proto_flags, fmt_hex);
+
 static ssize_t phys_port_id_show(struct device *dev,
 				 struct device_attribute *attr, char *buf)
 {
@@ -501,6 +514,7 @@ static struct attribute *net_class_attrs[] = {
 	&dev_attr_phys_port_id.attr,
 	&dev_attr_phys_port_name.attr,
 	&dev_attr_phys_switch_id.attr,
+	&dev_attr_proto_flags.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(net_class);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v4 1/4] net core: Add protodown support.
  2015-07-08 21:04 [PATCH net-next v4 1/4] net core: Add protodown support anuradhak
@ 2015-07-08 23:07 ` Jonathan Toppins
  2015-07-09  1:08   ` Anuradha Karuppiah
  2015-07-15 17:31 ` Stephen Hemminger
  1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Toppins @ 2015-07-08 23:07 UTC (permalink / raw
  To: anuradhak, davem, sfeldma; +Cc: netdev, roopa, gospo, wkok

On 7/8/15 5:04 PM, anuradhak@cumulusnetworks.com wrote:
> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> index 9cf2394..8d60fe7 100644
> --- a/include/uapi/linux/if.h
> +++ b/include/uapi/linux/if.h
> @@ -156,6 +156,12 @@ enum {
>   	IF_LINK_MODE_DORMANT,	/* limit upward transition to dormant */
>   };
>
> +/* proto_flags - port state information can be passed to the switch driver and
> + * used to determine the phys state of the switch port */
> +enum {
> +	IF_PROTOF_DOWN		= 1<<0	/* set switch port phys state down */

It might be better to use BIT(0) provided by linux/bitopts.h

> +};
> +

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v4 1/4] net core: Add protodown support.
  2015-07-08 23:07 ` Jonathan Toppins
@ 2015-07-09  1:08   ` Anuradha Karuppiah
  0 siblings, 0 replies; 5+ messages in thread
From: Anuradha Karuppiah @ 2015-07-09  1:08 UTC (permalink / raw
  To: Jonathan Toppins
  Cc: David S. Miller, Scott Feldman, netdev@vger.kernel.org,
	Roopa Prabhu, Andy Gospodarek, Wilson Kok

On Wed, Jul 8, 2015 at 4:07 PM, Jonathan Toppins
<jtoppins@cumulusnetworks.com> wrote:
> On 7/8/15 5:04 PM, anuradhak@cumulusnetworks.com wrote:
>>
>> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
>> index 9cf2394..8d60fe7 100644
>> --- a/include/uapi/linux/if.h
>> +++ b/include/uapi/linux/if.h
>> @@ -156,6 +156,12 @@ enum {
>>         IF_LINK_MODE_DORMANT,   /* limit upward transition to dormant */
>>   };
>>
>> +/* proto_flags - port state information can be passed to the switch
>> driver and
>> + * used to determine the phys state of the switch port */
>> +enum {
>> +       IF_PROTOF_DOWN          = 1<<0  /* set switch port phys state down
>> */
>
>
> It might be better to use BIT(0) provided by linux/bitopts.h
>
>> +};
>> +
>
>
Ack.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v4 1/4] net core: Add protodown support.
  2015-07-08 21:04 [PATCH net-next v4 1/4] net core: Add protodown support anuradhak
  2015-07-08 23:07 ` Jonathan Toppins
@ 2015-07-15 17:31 ` Stephen Hemminger
  2015-07-15 18:05   ` Anuradha Karuppiah
  1 sibling, 1 reply; 5+ messages in thread
From: Stephen Hemminger @ 2015-07-15 17:31 UTC (permalink / raw
  To: anuradhak; +Cc: davem, sfeldma, netdev, roopa, gospo, wkok

On Wed,  8 Jul 2015 14:04:22 -0700
anuradhak@cumulusnetworks.com wrote:

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index e20979d..99ebb01 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1041,6 +1041,11 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>   *	TX queue.
>   * int (*ndo_get_iflink)(const struct net_device *dev);
>   *	Called to get the iflink value of this device.
> + * void (*ndo_change_proto_flags)(struct net_device *dev,
> + *				  unsigned int proto_flags);
> + *	This function is used to pass protocol port state information
> + *	to the switch driver.
> + *
>   */
>  struct net_device_ops {
>  	int			(*ndo_init)(struct net_device *dev);
> @@ -1211,6 +1216,8 @@ struct net_device_ops {
>  						      int queue_index,
>  						      u32 maxrate);
>  	int			(*ndo_get_iflink)(const struct net_device *dev);
> +	int			(*ndo_change_proto_flags)(struct net_device *dev,
> +							  unsigned int proto_flags);
>  };
>  
>  /**
> @@ -1502,6 +1509,10 @@ enum netdev_priv_flags {
>   *
>   *	@qdisc_tx_busylock:	XXX: need comments on this one
>   *
> + *	@proto_flags:	protocol port state information can be sent to the
> + *			switch driver and used to set the phys state of the
> + *			switch port.
> + *
>   *	FIXME: cleanup struct net_device such that network protocol info
>   *	moves out.
>   */
> @@ -1762,6 +1773,7 @@ struct net_device {
>  #endif
>  	struct phy_device *phydev;
>  	struct lock_class_key *qdisc_tx_busylock;
> +	unsigned int proto_flags;
>  };
>  #define to_net_dev(d) container_of(d, struct net_device, dev)
>  
> @@ -2982,6 +2994,7 @@ int dev_get_phys_port_id(struct net_device *dev,
>  			 struct netdev_phys_item_id *ppid);
>  int dev_get_phys_port_name(struct net_device *dev,
>  			   char *name, size_t len);
> +int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags);
>  struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
>  struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>  				    struct netdev_queue *txq, int *ret);
> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
> index 9cf2394..8d60fe7 100644
> --- a/include/uapi/linux/if.h
> +++ b/include/uapi/linux/if.h
> @@ -156,6 +156,12 @@ enum {
>  	IF_LINK_MODE_DORMANT,	/* limit upward transition to dormant */
>  };
>  
> +/* proto_flags - port state information can be passed to the switch driver and
> + * used to determine the phys state of the switch port */
> +enum {
> +	IF_PROTOF_DOWN		= 1<<0	/* set switch port phys state down */
> +};
> +
>  /*
>   *	Device mapping structure. I'd just gone off and designed a 
>   *	beautiful scheme using only loadable modules with arguments
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6778a99..87571c4 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -6076,6 +6076,26 @@ int dev_get_phys_port_name(struct net_device *dev,
>  EXPORT_SYMBOL(dev_get_phys_port_name);
>  
>  /**
> + *	dev_change_proto_flags - update protocol port state information
> + *	@dev: device
> + *	@proto_flags: new value
> + *
> + *	This info can be used by switch drivers to set the phys state of the
> + *	port.
> + */
> +int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if (dev->proto_flags == proto_flags)
> +		return 0;
> +	if (!ops->ndo_change_proto_flags)
> +		return -EOPNOTSUPP;
> +	return ops->ndo_change_proto_flags(dev, proto_flags);
> +}
> +EXPORT_SYMBOL(dev_change_proto_flags);

Rather than introducing yet another ndo op etc, can't this be handled under some other
event (like change flags)?  It seems that this one small feature has grown to have bigger
impact than necessary.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v4 1/4] net core: Add protodown support.
  2015-07-15 17:31 ` Stephen Hemminger
@ 2015-07-15 18:05   ` Anuradha Karuppiah
  0 siblings, 0 replies; 5+ messages in thread
From: Anuradha Karuppiah @ 2015-07-15 18:05 UTC (permalink / raw
  To: Stephen Hemminger
  Cc: David S. Miller, Scott Feldman, netdev@vger.kernel.org,
	Roopa Prabhu, Andy Gospodarek, Wilson Kok

On Wed, Jul 15, 2015 at 10:31 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Wed,  8 Jul 2015 14:04:22 -0700
> anuradhak@cumulusnetworks.com wrote:
>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index e20979d..99ebb01 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1041,6 +1041,11 @@ typedef u16 (*select_queue_fallback_t)(struct net_device *dev,
>>   *   TX queue.
>>   * int (*ndo_get_iflink)(const struct net_device *dev);
>>   *   Called to get the iflink value of this device.
>> + * void (*ndo_change_proto_flags)(struct net_device *dev,
>> + *                             unsigned int proto_flags);
>> + *   This function is used to pass protocol port state information
>> + *   to the switch driver.
>> + *
>>   */
>>  struct net_device_ops {
>>       int                     (*ndo_init)(struct net_device *dev);
>> @@ -1211,6 +1216,8 @@ struct net_device_ops {
>>                                                     int queue_index,
>>                                                     u32 maxrate);
>>       int                     (*ndo_get_iflink)(const struct net_device *dev);
>> +     int                     (*ndo_change_proto_flags)(struct net_device *dev,
>> +                                                       unsigned int proto_flags);
>>  };
>>
>>  /**
>> @@ -1502,6 +1509,10 @@ enum netdev_priv_flags {
>>   *
>>   *   @qdisc_tx_busylock:     XXX: need comments on this one
>>   *
>> + *   @proto_flags:   protocol port state information can be sent to the
>> + *                   switch driver and used to set the phys state of the
>> + *                   switch port.
>> + *
>>   *   FIXME: cleanup struct net_device such that network protocol info
>>   *   moves out.
>>   */
>> @@ -1762,6 +1773,7 @@ struct net_device {
>>  #endif
>>       struct phy_device *phydev;
>>       struct lock_class_key *qdisc_tx_busylock;
>> +     unsigned int proto_flags;
>>  };
>>  #define to_net_dev(d) container_of(d, struct net_device, dev)
>>
>> @@ -2982,6 +2994,7 @@ int dev_get_phys_port_id(struct net_device *dev,
>>                        struct netdev_phys_item_id *ppid);
>>  int dev_get_phys_port_name(struct net_device *dev,
>>                          char *name, size_t len);
>> +int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags);
>>  struct sk_buff *validate_xmit_skb_list(struct sk_buff *skb, struct net_device *dev);
>>  struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
>>                                   struct netdev_queue *txq, int *ret);
>> diff --git a/include/uapi/linux/if.h b/include/uapi/linux/if.h
>> index 9cf2394..8d60fe7 100644
>> --- a/include/uapi/linux/if.h
>> +++ b/include/uapi/linux/if.h
>> @@ -156,6 +156,12 @@ enum {
>>       IF_LINK_MODE_DORMANT,   /* limit upward transition to dormant */
>>  };
>>
>> +/* proto_flags - port state information can be passed to the switch driver and
>> + * used to determine the phys state of the switch port */
>> +enum {
>> +     IF_PROTOF_DOWN          = 1<<0  /* set switch port phys state down */
>> +};
>> +
>>  /*
>>   *   Device mapping structure. I'd just gone off and designed a
>>   *   beautiful scheme using only loadable modules with arguments
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index 6778a99..87571c4 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -6076,6 +6076,26 @@ int dev_get_phys_port_name(struct net_device *dev,
>>  EXPORT_SYMBOL(dev_get_phys_port_name);
>>
>>  /**
>> + *   dev_change_proto_flags - update protocol port state information
>> + *   @dev: device
>> + *   @proto_flags: new value
>> + *
>> + *   This info can be used by switch drivers to set the phys state of the
>> + *   port.
>> + */
>> +int dev_change_proto_flags(struct net_device *dev, unsigned int proto_flags)
>> +{
>> +     const struct net_device_ops *ops = dev->netdev_ops;
>> +
>> +     if (dev->proto_flags == proto_flags)
>> +             return 0;
>> +     if (!ops->ndo_change_proto_flags)
>> +             return -EOPNOTSUPP;
>> +     return ops->ndo_change_proto_flags(dev, proto_flags);
>> +}
>> +EXPORT_SYMBOL(dev_change_proto_flags);
>
> Rather than introducing yet another ndo op etc, can't this be handled under some other
> event (like change flags)?  It seems that this one small feature has grown to have bigger
> impact than necessary.
>
Yes, I really wanted to avoid another ndo as well. The advantage of
adding the ndo was to isolate functionality and verify if the
operation was actually performed.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-07-15 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-08 21:04 [PATCH net-next v4 1/4] net core: Add protodown support anuradhak
2015-07-08 23:07 ` Jonathan Toppins
2015-07-09  1:08   ` Anuradha Karuppiah
2015-07-15 17:31 ` Stephen Hemminger
2015-07-15 18:05   ` Anuradha Karuppiah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).