All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
@ 2016-06-14 13:43 zyjzyj2000
  2016-06-14 13:43 ` [PATCH " zyjzyj2000
  0 siblings, 1 reply; 10+ messages in thread
From: zyjzyj2000 @ 2016-06-14 13:43 UTC (permalink / raw)
  To: e1000-devel, netdev


Hi, all

When the fiber tranceiver is plugged/unplugged from the nic, there is no any notifiers to indicate this events now.
This events sometimes are needed by the userspace tools or kernel.

So, 

Is there any interrupt to indicate the unplug event?
If no interrupt, is there any method to notify the unplug event?

I made a new patch to send the notifier when the fiber tranceiver is plugged/unplugged.

This patch is based on the function ixgbe_sfp_detection_subtask.

When the fiber tranceiver is plugged/unplugged, the value in the register is changed, then
NETDEV_FIBER_TRANCEIVER_UNPLUG notifier is sent.

This patch can work well with the following steps:

1. boot the host
2. ip link set eth0 up
3. unplug the fiber tranceiver
4. a message NETDEV_FIBER_TRANCEIVER_UNPLUG is sent
5. plug the fiber tranceiver
6. a notifier NETDEV_FIBER_TRANCEIVER_PLUG is sent

Please comment on it.

Thanks a lot.
Zhu Yanjun

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

* [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-14 13:43 [PATCH net-next 1/1] ixgbe: add fiber tranceiver plug/unplug notifier zyjzyj2000
@ 2016-06-14 13:43 ` zyjzyj2000
  2016-06-14 13:55   ` zhuyj
  0 siblings, 1 reply; 10+ messages in thread
From: zyjzyj2000 @ 2016-06-14 13:43 UTC (permalink / raw)
  To: e1000-devel, netdev; +Cc: Zhu Yanjun

From: Zhu Yanjun <zyjzyj2000@gmail.com>

When the fiber tranceiver is plugged/unplugged, a netdev notifier is
sent. The userspace tools or kernel can receive this notifier.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   26 ++++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 +++
 include/linux/netdevice.h                     |    2 ++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index cb19cbc..df0eed3 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	hw->revision_id = pdev->revision;
 	hw->subsystem_vendor_id = pdev->subsystem_vendor;
 	hw->subsystem_device_id = pdev->subsystem_device;
+	hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
+	hw->tranceiver_polltime = 0;
 
 	/* Set common capability flags and settings */
 	rss = min_t(int, ixgbe_max_rss_indices(adapter), num_online_cpus());
@@ -7067,7 +7069,25 @@ static void ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)
 static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	s32 err;
+	s32 err, status;
+
+	if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
+	    time_after(jiffies, hw->tranceiver_polltime)) {
+		u8 identifier;
+
+		status = hw->phy.ops.read_i2c_eeprom(hw,
+						     0x0,
+						     &identifier);
+		if ((status != hw->last_tranceiver_status) && status) {
+			hw->phy.sfp_type = ixgbe_sfp_type_not_present;
+			rtnl_lock();
+			call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_UNPLUG,
+						 adapter->netdev);
+			rtnl_unlock();
+		}
+		hw->last_tranceiver_status = status;
+		hw->tranceiver_polltime = jiffies + 3 * HZ;
+	}
 
 	/* If crosstalk fix enabled verify the SFP+ cage is full */
 	if (adapter->need_crosstalk_fix) {
@@ -7130,6 +7150,10 @@ static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
 	adapter->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
 	e_info(probe, "detected SFP+: %d\n", hw->phy.sfp_type);
 
+	rtnl_lock();
+	call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_PLUG, adapter->netdev);
+	rtnl_unlock();
+
 sfp_out:
 	clear_bit(__IXGBE_IN_SFP_INIT, &adapter->state);
 
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index da3d835..0dde95c 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3525,6 +3525,8 @@ struct ixgbe_hw {
 	bool				force_full_reset;
 	bool				allow_unsupported_sfp;
 	bool				wol_enabled;
+	s32				last_tranceiver_status;
+	unsigned long			tranceiver_polltime;
 };
 
 struct ixgbe_info {
@@ -3574,6 +3576,7 @@ struct ixgbe_info {
 #define IXGBE_ERR_FDIR_CMD_INCOMPLETE		-38
 #define IXGBE_ERR_FW_RESP_INVALID		-39
 #define IXGBE_ERR_TOKEN_RETRY			-40
+#define IXGBE_ERR_TRANCEIVER_NOT_PRESENT	-41
 #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
 
 #define IXGBE_FUSES0_GROUP(_i)		(0x11158 + ((_i) * 4))
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d101e4d..693ba92 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2253,6 +2253,8 @@ struct netdev_lag_lower_state_info {
 #define NETDEV_CHANGELOWERSTATE	0x001B
 #define NETDEV_OFFLOAD_PUSH_VXLAN	0x001C
 #define NETDEV_OFFLOAD_PUSH_GENEVE	0x001D
+#define NETDEV_FIBER_TRANCEIVER_PLUG	0x001E
+#define NETDEV_FIBER_TRANCEIVER_UNPLUG	0x001F
 
 int register_netdevice_notifier(struct notifier_block *nb);
 int unregister_netdevice_notifier(struct notifier_block *nb);
-- 
1.7.9.5

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

* Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-14 13:43 ` [PATCH " zyjzyj2000
@ 2016-06-14 13:55   ` zhuyj
  2016-06-14 15:20     ` Skidmore, Donald C
  0 siblings, 1 reply; 10+ messages in thread
From: zhuyj @ 2016-06-14 13:55 UTC (permalink / raw)
  To: e1000-devel, netdev


[-- Attachment #1.1: Type: text/plain, Size: 5568 bytes --]

Hi, all

When the fiber tranceiver is plugged/unplugged from the nic, there is no
any notifier to indicate this event now.
This event sometimes is needed by the userspace tools or kernel.

So,

Is there any interrupt to indicate this event?
If no interrupt, is there any method to notify this event to the netdev
notifier chain?

I made a new patch to send the notifier when the fiber tranceiver is
plugged/unplugged.

This patch is based on the function ixgbe_sfp_detection_subtask.

When the fiber tranceiver is plugged/unplugged, the value in the register
is changed, then
NETDEV_FIBER_TRANCEIVER_UNPLUG/NETDEV_FIBER_TRANCEIVER_PLUG notifier is
sent.

This patch can work well with the following steps:

1. boot the host
2. ip link set eth0 up
3. unplug the fiber tranceiver
4. a message NETDEV_FIBER_TRANCEIVER_UNPLUG is sent
5. plug the fiber tranceiver
6. a notifier NETDEV_FIBER_TRANCEIVER_PLUG is sent

Please comment on it.

On Tue, Jun 14, 2016 at 9:43 PM, <zyjzyj2000@gmail.com> wrote:

> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
> When the fiber tranceiver is plugged/unplugged, a netdev notifier is
> sent. The userspace tools or kernel can receive this notifier.
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   26
> ++++++++++++++++++++++++-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 +++
>  include/linux/netdevice.h                     |    2 ++
>  3 files changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index cb19cbc..df0eed3 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter
> *adapter)
>         hw->revision_id = pdev->revision;
>         hw->subsystem_vendor_id = pdev->subsystem_vendor;
>         hw->subsystem_device_id = pdev->subsystem_device;
> +       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
> +       hw->tranceiver_polltime = 0;
>
>         /* Set common capability flags and settings */
>         rss = min_t(int, ixgbe_max_rss_indices(adapter),
> num_online_cpus());
> @@ -7067,7 +7069,25 @@ static void ixgbe_watchdog_subtask(struct
> ixgbe_adapter *adapter)
>  static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
>  {
>         struct ixgbe_hw *hw = &adapter->hw;
> -       s32 err;
> +       s32 err, status;
> +
> +       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
> +           time_after(jiffies, hw->tranceiver_polltime)) {
> +               u8 identifier;
> +
> +               status = hw->phy.ops.read_i2c_eeprom(hw,
> +                                                    0x0,
> +                                                    &identifier);
> +               if ((status != hw->last_tranceiver_status) && status) {
> +                       hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> +                       rtnl_lock();
> +
>  call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_UNPLUG,
> +                                                adapter->netdev);
> +                       rtnl_unlock();
> +               }
> +               hw->last_tranceiver_status = status;
> +               hw->tranceiver_polltime = jiffies + 3 * HZ;
> +       }
>
>         /* If crosstalk fix enabled verify the SFP+ cage is full */
>         if (adapter->need_crosstalk_fix) {
> @@ -7130,6 +7150,10 @@ static void ixgbe_sfp_detection_subtask(struct
> ixgbe_adapter *adapter)
>         adapter->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
>         e_info(probe, "detected SFP+: %d\n", hw->phy.sfp_type);
>
> +       rtnl_lock();
> +       call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_PLUG,
> adapter->netdev);
> +       rtnl_unlock();
> +
>  sfp_out:
>         clear_bit(__IXGBE_IN_SFP_INIT, &adapter->state);
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index da3d835..0dde95c 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3525,6 +3525,8 @@ struct ixgbe_hw {
>         bool                            force_full_reset;
>         bool                            allow_unsupported_sfp;
>         bool                            wol_enabled;
> +       s32                             last_tranceiver_status;
> +       unsigned long                   tranceiver_polltime;
>  };
>
>  struct ixgbe_info {
> @@ -3574,6 +3576,7 @@ struct ixgbe_info {
>  #define IXGBE_ERR_FDIR_CMD_INCOMPLETE          -38
>  #define IXGBE_ERR_FW_RESP_INVALID              -39
>  #define IXGBE_ERR_TOKEN_RETRY                  -40
> +#define IXGBE_ERR_TRANCEIVER_NOT_PRESENT       -41
>  #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
>
>  #define IXGBE_FUSES0_GROUP(_i)         (0x11158 + ((_i) * 4))
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index d101e4d..693ba92 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -2253,6 +2253,8 @@ struct netdev_lag_lower_state_info {
>  #define NETDEV_CHANGELOWERSTATE        0x001B
>  #define NETDEV_OFFLOAD_PUSH_VXLAN      0x001C
>  #define NETDEV_OFFLOAD_PUSH_GENEVE     0x001D
> +#define NETDEV_FIBER_TRANCEIVER_PLUG   0x001E
> +#define NETDEV_FIBER_TRANCEIVER_UNPLUG 0x001F
>
>  int register_netdevice_notifier(struct notifier_block *nb);
>  int unregister_netdevice_notifier(struct notifier_block *nb);
> --
> 1.7.9.5
>
>

[-- Attachment #2: Type: text/plain, Size: 453 bytes --]

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

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

* Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-14 13:55   ` zhuyj
@ 2016-06-14 15:20     ` Skidmore, Donald C
  2016-06-14 15:54       ` [E1000-devel] " Stephen Hemminger
  2016-06-15 13:36       ` [PATCH net-next " zyjzyj2000
  0 siblings, 2 replies; 10+ messages in thread
From: Skidmore, Donald C @ 2016-06-14 15:20 UTC (permalink / raw)
  To: zhuyj, e1000-devel, netdev

> -----Original Message-----
> From: zhuyj [mailto:zyjzyj2000@gmail.com]
> Sent: Tuesday, June 14, 2016 6:55 AM
> To: e1000-devel@lists.sourceforge.net; netdev <netdev@vger.kernel.org>
> Subject: Re: [E1000-devel] [PATCH 1/1] ixgbe: add fiber tranceiver
> plug/unplug notifier
> 
> Hi, all
> 
> When the fiber tranceiver is plugged/unplugged from the nic, there is no any
> notifier to indicate this event now.
> This event sometimes is needed by the userspace tools or kernel.
> 
> So,
> 
> Is there any interrupt to indicate this event?
> If no interrupt, is there any method to notify this event to the netdev notifier
> chain?
> 
> I made a new patch to send the notifier when the fiber tranceiver is
> plugged/unplugged.
> 
> This patch is based on the function ixgbe_sfp_detection_subtask.
> 
> When the fiber tranceiver is plugged/unplugged, the value in the register is
> changed, then
> NETDEV_FIBER_TRANCEIVER_UNPLUG/NETDEV_FIBER_TRANCEIVER_PLUG
> notifier is sent.
> 
> This patch can work well with the following steps:
> 
> 1. boot the host
> 2. ip link set eth0 up
> 3. unplug the fiber tranceiver
> 4. a message NETDEV_FIBER_TRANCEIVER_UNPLUG is sent 5. plug the fiber
> tranceiver 6. a notifier NETDEV_FIBER_TRANCEIVER_PLUG is sent
> 
> Please comment on it.
> 
> On Tue, Jun 14, 2016 at 9:43 PM, <zyjzyj2000@gmail.com> wrote:
> 
> > From: Zhu Yanjun <zyjzyj2000@gmail.com>
> >
> > When the fiber tranceiver is plugged/unplugged, a netdev notifier is
> > sent. The userspace tools or kernel can receive this notifier.
> >
> > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> > ---
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   26
> > ++++++++++++++++++++++++-
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 +++
> >  include/linux/netdevice.h                     |    2 ++
> >  3 files changed, 30 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > index cb19cbc..df0eed3 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > @@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter
> > *adapter)
> >         hw->revision_id = pdev->revision;
> >         hw->subsystem_vendor_id = pdev->subsystem_vendor;
> >         hw->subsystem_device_id = pdev->subsystem_device;
> > +       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
> > +       hw->tranceiver_polltime = 0;
> >
> >         /* Set common capability flags and settings */
> >         rss = min_t(int, ixgbe_max_rss_indices(adapter),
> > num_online_cpus()); @@ -7067,7 +7069,25 @@ static void
> > ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)  static void
> > ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)  {
> >         struct ixgbe_hw *hw = &adapter->hw;
> > -       s32 err;
> > +       s32 err, status;
> > +
> > +       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber)
> &&
> > +           time_after(jiffies, hw->tranceiver_polltime)) {
> > +               u8 identifier;
> > +
> > +               status = hw->phy.ops.read_i2c_eeprom(hw,
> > +                                                    0x0,
> > +                                                    &identifier);
> > +               if ((status != hw->last_tranceiver_status) && status) {
> > +                       hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > +                       rtnl_lock();
> > +
> >  call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_UNPLUG,
> > +                                                adapter->netdev);
> > +                       rtnl_unlock();
> > +               }
> > +               hw->last_tranceiver_status = status;
> > +               hw->tranceiver_polltime = jiffies + 3 * HZ;
> > +       }
> >
> >         /* If crosstalk fix enabled verify the SFP+ cage is full */
> >         if (adapter->need_crosstalk_fix) { @@ -7130,6 +7150,10 @@
> > static void ixgbe_sfp_detection_subtask(struct
> > ixgbe_adapter *adapter)
> >         adapter->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> >         e_info(probe, "detected SFP+: %d\n", hw->phy.sfp_type);
> >
> > +       rtnl_lock();
> > +       call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_PLUG,
> > adapter->netdev);
> > +       rtnl_unlock();
> > +
> >  sfp_out:
> >         clear_bit(__IXGBE_IN_SFP_INIT, &adapter->state);
> >
> > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > index da3d835..0dde95c 100644
> > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > @@ -3525,6 +3525,8 @@ struct ixgbe_hw {
> >         bool                            force_full_reset;
> >         bool                            allow_unsupported_sfp;
> >         bool                            wol_enabled;
> > +       s32                             last_tranceiver_status;
> > +       unsigned long                   tranceiver_polltime;
> >  };
> >
> >  struct ixgbe_info {
> > @@ -3574,6 +3576,7 @@ struct ixgbe_info {
> >  #define IXGBE_ERR_FDIR_CMD_INCOMPLETE          -38
> >  #define IXGBE_ERR_FW_RESP_INVALID              -39
> >  #define IXGBE_ERR_TOKEN_RETRY                  -40
> > +#define IXGBE_ERR_TRANCEIVER_NOT_PRESENT       -41
> >  #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
> >
> >  #define IXGBE_FUSES0_GROUP(_i)         (0x11158 + ((_i) * 4))
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index d101e4d..693ba92 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -2253,6 +2253,8 @@ struct netdev_lag_lower_state_info {
> >  #define NETDEV_CHANGELOWERSTATE        0x001B
> >  #define NETDEV_OFFLOAD_PUSH_VXLAN      0x001C
> >  #define NETDEV_OFFLOAD_PUSH_GENEVE     0x001D
> > +#define NETDEV_FIBER_TRANCEIVER_PLUG   0x001E
> > +#define NETDEV_FIBER_TRANCEIVER_UNPLUG 0x001F
> >
> >  int register_netdevice_notifier(struct notifier_block *nb);  int
> > unregister_netdevice_notifier(struct notifier_block *nb);
> > --
> > 1.7.9.5
> >
> >

It's not as simple as this patch is implying.  The ixgbe_sfp_detection_subtask function is driven by our master service task which isn't always running and doesn't always run at the same frequency.  Likewise the all hardware supported by ixgbe doesn't all behave the same way in relation to a transceiver insertion/removal event.  Some receive interrupts while others have to poll.  Off the top of my head I think you would also have to be concerned with the following cases:

- Insertion/removal while the device is down.
- Initial query when the driver is loaded.
- Some modules aren't supported I'm not sure how you would want to there, still report the insertion to the stack, ignore it?
- I've also recently added a crosstalk fix that your patch would not take in to consideration.

Thanks,
-Don Skidmore <donald.c.skidmore@intel.com>

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

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

* Re: [E1000-devel] [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-14 15:20     ` Skidmore, Donald C
@ 2016-06-14 15:54       ` Stephen Hemminger
  2016-06-15 13:36       ` [PATCH net-next " zyjzyj2000
  1 sibling, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2016-06-14 15:54 UTC (permalink / raw)
  To: Skidmore, Donald C; +Cc: zhuyj, e1000-devel, netdev

On Tue, 14 Jun 2016 15:20:07 +0000
"Skidmore, Donald C" <donald.c.skidmore@intel.com> wrote:

> > -----Original Message-----
> > From: zhuyj [mailto:zyjzyj2000@gmail.com]
> > Sent: Tuesday, June 14, 2016 6:55 AM
> > To: e1000-devel@lists.sourceforge.net; netdev <netdev@vger.kernel.org>
> > Subject: Re: [E1000-devel] [PATCH 1/1] ixgbe: add fiber tranceiver
> > plug/unplug notifier
> > 
> > Hi, all
> > 
> > When the fiber tranceiver is plugged/unplugged from the nic, there is no any
> > notifier to indicate this event now.
> > This event sometimes is needed by the userspace tools or kernel.
> > 
> > So,
> > 
> > Is there any interrupt to indicate this event?
> > If no interrupt, is there any method to notify this event to the netdev notifier
> > chain?
> > 
> > I made a new patch to send the notifier when the fiber tranceiver is
> > plugged/unplugged.
> > 
> > This patch is based on the function ixgbe_sfp_detection_subtask.
> > 
> > When the fiber tranceiver is plugged/unplugged, the value in the register is
> > changed, then
> > NETDEV_FIBER_TRANCEIVER_UNPLUG/NETDEV_FIBER_TRANCEIVER_PLUG
> > notifier is sent.
> > 
> > This patch can work well with the following steps:
> > 
> > 1. boot the host
> > 2. ip link set eth0 up
> > 3. unplug the fiber tranceiver
> > 4. a message NETDEV_FIBER_TRANCEIVER_UNPLUG is sent 5. plug the fiber
> > tranceiver 6. a notifier NETDEV_FIBER_TRANCEIVER_PLUG is sent
> > 
> > Please comment on it.
> > 
> > On Tue, Jun 14, 2016 at 9:43 PM, <zyjzyj2000@gmail.com> wrote:
> > 
> > > From: Zhu Yanjun <zyjzyj2000@gmail.com>
> > >
> > > When the fiber tranceiver is plugged/unplugged, a netdev notifier is
> > > sent. The userspace tools or kernel can receive this notifier.
> > >
> > > Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> > > ---
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   26
> > > ++++++++++++++++++++++++-
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    3 +++
> > >  include/linux/netdevice.h                     |    2 ++
> > >  3 files changed, 30 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > index cb19cbc..df0eed3 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> > > @@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter
> > > *adapter)
> > >         hw->revision_id = pdev->revision;
> > >         hw->subsystem_vendor_id = pdev->subsystem_vendor;
> > >         hw->subsystem_device_id = pdev->subsystem_device;
> > > +       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
> > > +       hw->tranceiver_polltime = 0;
> > >
> > >         /* Set common capability flags and settings */
> > >         rss = min_t(int, ixgbe_max_rss_indices(adapter),
> > > num_online_cpus()); @@ -7067,7 +7069,25 @@ static void
> > > ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)  static void
> > > ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)  {
> > >         struct ixgbe_hw *hw = &adapter->hw;
> > > -       s32 err;
> > > +       s32 err, status;
> > > +
> > > +       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber)
> > &&
> > > +           time_after(jiffies, hw->tranceiver_polltime)) {
> > > +               u8 identifier;
> > > +
> > > +               status = hw->phy.ops.read_i2c_eeprom(hw,
> > > +                                                    0x0,
> > > +                                                    &identifier);
> > > +               if ((status != hw->last_tranceiver_status) && status) {
> > > +                       hw->phy.sfp_type = ixgbe_sfp_type_not_present;
> > > +                       rtnl_lock();
> > > +
> > >  call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_UNPLUG,
> > > +                                                adapter->netdev);
> > > +                       rtnl_unlock();
> > > +               }
> > > +               hw->last_tranceiver_status = status;
> > > +               hw->tranceiver_polltime = jiffies + 3 * HZ;
> > > +       }
> > >
> > >         /* If crosstalk fix enabled verify the SFP+ cage is full */
> > >         if (adapter->need_crosstalk_fix) { @@ -7130,6 +7150,10 @@
> > > static void ixgbe_sfp_detection_subtask(struct
> > > ixgbe_adapter *adapter)
> > >         adapter->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> > >         e_info(probe, "detected SFP+: %d\n", hw->phy.sfp_type);
> > >
> > > +       rtnl_lock();
> > > +       call_netdevice_notifiers(NETDEV_FIBER_TRANCEIVER_PLUG,
> > > adapter->netdev);
> > > +       rtnl_unlock();
> > > +
> > >  sfp_out:
> > >         clear_bit(__IXGBE_IN_SFP_INIT, &adapter->state);
> > >
> > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > > b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > > index da3d835..0dde95c 100644
> > > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> > > @@ -3525,6 +3525,8 @@ struct ixgbe_hw {
> > >         bool                            force_full_reset;
> > >         bool                            allow_unsupported_sfp;
> > >         bool                            wol_enabled;
> > > +       s32                             last_tranceiver_status;
> > > +       unsigned long                   tranceiver_polltime;
> > >  };
> > >
> > >  struct ixgbe_info {
> > > @@ -3574,6 +3576,7 @@ struct ixgbe_info {
> > >  #define IXGBE_ERR_FDIR_CMD_INCOMPLETE          -38
> > >  #define IXGBE_ERR_FW_RESP_INVALID              -39
> > >  #define IXGBE_ERR_TOKEN_RETRY                  -40
> > > +#define IXGBE_ERR_TRANCEIVER_NOT_PRESENT       -41
> > >  #define IXGBE_NOT_IMPLEMENTED                   0x7FFFFFFF
> > >
> > >  #define IXGBE_FUSES0_GROUP(_i)         (0x11158 + ((_i) * 4))
> > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > index d101e4d..693ba92 100644
> > > --- a/include/linux/netdevice.h
> > > +++ b/include/linux/netdevice.h
> > > @@ -2253,6 +2253,8 @@ struct netdev_lag_lower_state_info {
> > >  #define NETDEV_CHANGELOWERSTATE        0x001B
> > >  #define NETDEV_OFFLOAD_PUSH_VXLAN      0x001C
> > >  #define NETDEV_OFFLOAD_PUSH_GENEVE     0x001D
> > > +#define NETDEV_FIBER_TRANCEIVER_PLUG   0x001E
> > > +#define NETDEV_FIBER_TRANCEIVER_UNPLUG 0x001F
> > >
> > >  int register_netdevice_notifier(struct notifier_block *nb);  int
> > > unregister_netdevice_notifier(struct notifier_block *nb);
> > > --
> > > 1.7.9.5
> > >
> > >
> 
> It's not as simple as this patch is implying.  The ixgbe_sfp_detection_subtask function is driven by our master service task which isn't always running and doesn't always run at the same frequency.  Likewise the all hardware supported by ixgbe doesn't all behave the same way in relation to a transceiver insertion/removal event.  Some receive interrupts while others have to poll.  Off the top of my head I think you would also have to be concerned with the following cases:
> 
> - Insertion/removal while the device is down.
> - Initial query when the driver is loaded.
> - Some modules aren't supported I'm not sure how you would want to there, still report the insertion to the stack, ignore it?
> - I've also recently added a crosstalk fix that your patch would not take in to consideration.
> 
> Thanks,
> -Don Skidmore <donald.c.skidmore@intel.com>

In theory you could use exiting network device model about operstate.
If SFP is removed this is analgous to LOWERLAYER down.

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

* [PATCH net-next 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-14 15:20     ` Skidmore, Donald C
  2016-06-14 15:54       ` [E1000-devel] " Stephen Hemminger
@ 2016-06-15 13:36       ` zyjzyj2000
  2016-06-15 13:36         ` [PATCH " zyjzyj2000
  1 sibling, 1 reply; 10+ messages in thread
From: zyjzyj2000 @ 2016-06-15 13:36 UTC (permalink / raw)
  To: e1000-devel, netdev, donald.c.skidmore


Hi, Don

The latest patch is based on your suggestion.
The changes are as below:

1. Replace IXGBE_IDENTIFIER with IXGBE_ESDP;
2. Replace plug interrupt with poll;
3. Indicate the NICs that does not support to plug/unplug tranceiver as plugged;

The patch can work well with the following steps:

 1. boot the host
 2. ip link set eth0 up
 3. unplug the fiber tranceiver
 4. a message NETDEV_FIBER_TRANCEIVER_UNPLUG is sent
 5. plug the fiber tranceiver
 6. a notifier NETDEV_FIBER_TRANCEIVER_PLUG is sent

Any reply is appreciated.

Zhu Yanun

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

* [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-15 13:36       ` [PATCH net-next " zyjzyj2000
@ 2016-06-15 13:36         ` zyjzyj2000
  2016-06-16 10:43           ` zhuyj
  0 siblings, 1 reply; 10+ messages in thread
From: zyjzyj2000 @ 2016-06-15 13:36 UTC (permalink / raw)
  To: e1000-devel, netdev, donald.c.skidmore; +Cc: Zhu Yanjun

From: Zhu Yanjun <zyjzyj2000@gmail.com>

When the fiber tranceiver is plugged/unplugged, a netdev notifier is
 sent. The userspace tools or kernel can receive this notifier.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24 +++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    2 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 088c47c..1d8c1ff 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	hw->revision_id = pdev->revision;
 	hw->subsystem_vendor_id = pdev->subsystem_vendor;
 	hw->subsystem_device_id = pdev->subsystem_device;
+	hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
+	hw->tranceiver_polltime = 0;
 
 	/* Set common capability flags and settings */
 	rss = min_t(int, ixgbe_max_rss_indices(adapter), num_online_cpus());
@@ -7067,7 +7069,27 @@ static void ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)
 static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
 {
 	struct ixgbe_hw *hw = &adapter->hw;
-	s32 err;
+	s32 err, status;
+
+	if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
+	    time_after(jiffies, hw->tranceiver_polltime)) {
+		status = IXGBE_READ_REG(hw, IXGBE_ESDP) & IXGBE_ESDP_SDP2;
+		if (status != hw->last_tranceiver_status) {
+			unsigned long val;
+
+			if (!status) {
+				hw->phy.sfp_type = ixgbe_sfp_type_not_present;
+				val = NETDEV_FIBER_TRANCEIVER_UNPLUG;
+			} else {
+				val = NETDEV_FIBER_TRANCEIVER_PLUG;
+			}
+			rtnl_lock();
+			call_netdevice_notifiers(val, adapter->netdev);
+			rtnl_unlock();
+		}
+		hw->last_tranceiver_status = status;
+		hw->tranceiver_polltime = jiffies + 3 * HZ;
+	}
 
 	/* If crosstalk fix enabled verify the SFP+ cage is full */
 	if (adapter->need_crosstalk_fix) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index da3d835..fe19899 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3525,6 +3525,8 @@ struct ixgbe_hw {
 	bool				force_full_reset;
 	bool				allow_unsupported_sfp;
 	bool				wol_enabled;
+	s32				last_tranceiver_status;
+	unsigned long			tranceiver_polltime;
 };
 
 struct ixgbe_info {
-- 
1.7.9.5

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

* Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-15 13:36         ` [PATCH " zyjzyj2000
@ 2016-06-16 10:43           ` zhuyj
  2016-06-16 17:46             ` Skidmore, Donald C
  0 siblings, 1 reply; 10+ messages in thread
From: zhuyj @ 2016-06-16 10:43 UTC (permalink / raw)
  To: e1000-devel, netdev, Skidmore, Donald C


[-- Attachment #1.1: Type: text/plain, Size: 3393 bytes --]

Sorry, Maybe last_tranceiver_status and tranceiver_polltime should not be
in ixgbe_hw struct. It is better in ixgbe struct. I will change it soon.

Zhu Yanjun

On Wed, Jun 15, 2016 at 9:36 PM, <zyjzyj2000@gmail.com> wrote:

> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
> When the fiber tranceiver is plugged/unplugged, a netdev notifier is
>  sent. The userspace tools or kernel can receive this notifier.
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24
> +++++++++++++++++++++++-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    2 ++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 088c47c..1d8c1ff 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter
> *adapter)
>         hw->revision_id = pdev->revision;
>         hw->subsystem_vendor_id = pdev->subsystem_vendor;
>         hw->subsystem_device_id = pdev->subsystem_device;
> +       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
> +       hw->tranceiver_polltime = 0;
>
>         /* Set common capability flags and settings */
>         rss = min_t(int, ixgbe_max_rss_indices(adapter),
> num_online_cpus());
> @@ -7067,7 +7069,27 @@ static void ixgbe_watchdog_subtask(struct
> ixgbe_adapter *adapter)
>  static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
>  {
>         struct ixgbe_hw *hw = &adapter->hw;
> -       s32 err;
> +       s32 err, status;
> +
> +       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
> +           time_after(jiffies, hw->tranceiver_polltime)) {
> +               status = IXGBE_READ_REG(hw, IXGBE_ESDP) & IXGBE_ESDP_SDP2;
> +               if (status != hw->last_tranceiver_status) {
> +                       unsigned long val;
> +
> +                       if (!status) {
> +                               hw->phy.sfp_type =
> ixgbe_sfp_type_not_present;
> +                               val = NETDEV_FIBER_TRANCEIVER_UNPLUG;
> +                       } else {
> +                               val = NETDEV_FIBER_TRANCEIVER_PLUG;
> +                       }
> +                       rtnl_lock();
> +                       call_netdevice_notifiers(val, adapter->netdev);
> +                       rtnl_unlock();
> +               }
> +               hw->last_tranceiver_status = status;
> +               hw->tranceiver_polltime = jiffies + 3 * HZ;
> +       }
>
>         /* If crosstalk fix enabled verify the SFP+ cage is full */
>         if (adapter->need_crosstalk_fix) {
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index da3d835..fe19899 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3525,6 +3525,8 @@ struct ixgbe_hw {
>         bool                            force_full_reset;
>         bool                            allow_unsupported_sfp;
>         bool                            wol_enabled;
> +       s32                             last_tranceiver_status;
> +       unsigned long                   tranceiver_polltime;
>  };
>
>  struct ixgbe_info {
> --
> 1.7.9.5
>
>

[-- Attachment #2: Type: text/plain, Size: 465 bytes --]

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports. http://pubads.g.doubleclick.net/gampad/clk?id=1444514421&iu=/41014381

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

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

* RE: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-16 10:43           ` zhuyj
@ 2016-06-16 17:46             ` Skidmore, Donald C
  2016-06-20  2:50               ` zhuyj
  0 siblings, 1 reply; 10+ messages in thread
From: Skidmore, Donald C @ 2016-06-16 17:46 UTC (permalink / raw)
  To: zhuyj, e1000-devel, netdev



From: zhuyj [mailto:zyjzyj2000@gmail.com] 
Sent: Thursday, June 16, 2016 3:43 AM
To: e1000-devel@lists.sourceforge.net; netdev <netdev@vger.kernel.org>; Skidmore, Donald C <donald.c.skidmore@intel.com>
Cc: Zhu Yanjun <zyjzyj2000@gmail.com>
Subject: Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier

Sorry, Maybe last_tranceiver_status and tranceiver_polltime should not be in ixgbe_hw struct. It is better in ixgbe struct. I will change it soon.

Zhu Yanjun

On Wed, Jun 15, 2016 at 9:36 PM, <zyjzyj2000@gmail.com> wrote:
From: Zhu Yanjun <zyjzyj2000@gmail.com>

When the fiber tranceiver is plugged/unplugged, a netdev notifier is
 sent. The userspace tools or kernel can receive this notifier.

Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24 +++++++++++++++++++++++-
 drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    2 ++
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 088c47c..1d8c1ff 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter *adapter)
        hw->revision_id = pdev->revision;
        hw->subsystem_vendor_id = pdev->subsystem_vendor;
        hw->subsystem_device_id = pdev->subsystem_device;
+       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
+       hw->tranceiver_polltime = 0;

        /* Set common capability flags and settings */
        rss = min_t(int, ixgbe_max_rss_indices(adapter), num_online_cpus());
@@ -7067,7 +7069,27 @@ static void ixgbe_watchdog_subtask(struct ixgbe_adapter *adapter)
 static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
 {
        struct ixgbe_hw *hw = &adapter->hw;
-       s32 err;
+       s32 err, status;
+
+       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
+           time_after(jiffies, hw->tranceiver_polltime)) {
+               status = IXGBE_READ_REG(hw, IXGBE_ESDP) & IXGBE_ESDP_SDP2;
+               if (status != hw->last_tranceiver_status) {
+                       unsigned long val;
+
+                       if (!status) {
+                               hw->phy.sfp_type = ixgbe_sfp_type_not_present;
+                               val = NETDEV_FIBER_TRANCEIVER_UNPLUG;
+                       } else {
+                               val = NETDEV_FIBER_TRANCEIVER_PLUG;
+                       }
+                       rtnl_lock();
+                       call_netdevice_notifiers(val, adapter->netdev);
+                       rtnl_unlock();
+               }
+               hw->last_tranceiver_status = status;
+               hw->tranceiver_polltime = jiffies + 3 * HZ;
+       }

        /* If crosstalk fix enabled verify the SFP+ cage is full */
        if (adapter->need_crosstalk_fix) {
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
index da3d835..fe19899 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
@@ -3525,6 +3525,8 @@ struct ixgbe_hw {
        bool                            force_full_reset;
        bool                            allow_unsupported_sfp;
        bool                            wol_enabled;
+       s32                             last_tranceiver_status;
+       unsigned long                   tranceiver_polltime;
 };

 struct ixgbe_info {
--
1.7.9.5


What exactly is this information (arrival/removal of a SFP+ module) needed for?  I can't think of any use the kernel would have, outside the driver.  So I must assume a userspace application, if so what is it?

Top that off this notification would only be actuate while the port was up, which limits its value even further.  The driver just doesn't bother keeping this information unless it is needed.   With these limitations I'm not seeing the advantage of adding this.

Also what a given SDP is connected to different for different HW types so assumptions made in the patch above would be incorrect on various platforms.

-Don Skidmore <donald.c.skidmore@intel.com>










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

* Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
  2016-06-16 17:46             ` Skidmore, Donald C
@ 2016-06-20  2:50               ` zhuyj
  0 siblings, 0 replies; 10+ messages in thread
From: zhuyj @ 2016-06-20  2:50 UTC (permalink / raw)
  To: Skidmore, Donald C; +Cc: e1000-devel, netdev


[-- Attachment #1.1: Type: text/plain, Size: 5749 bytes --]

Hi, Don

Thanks for your reply.
1. --- What exactly is this information (arrival/removal of a SFP+ module)
needed for?
Sure. A userspace application needs this. In a scenario, there are a lot of
ixgbe sfp nic in the hosts. This userspace application can monitor the
status of ixgbe sfp nic. If the cage is empty, the application can be
notified immediately.
In a word, it is easy to monitor the status of ixgbe sfp nic aftet this
patch is applied.

2. --- With these limitations I'm not seeing the advantage of adding this.
This notification is sent after this port is enabled. It is easy to do this
with the command "ip link set ethx up". And I made tests that the
notification is sent if there is some changes in the sfp nic status after
this port is enabled. This will help the administrator a lot.

Since the status is got from the register timely, it is not necessary to
keep this information in the driver.

3. Also what a given SDP is connected to different for different HW types
so assumptions made in the patch above would be incorrect on various
platforms.

Sorry. I do not understand the above clearly. Would you like to share an
example with me ?

Thanks a lot.
Any reply is appreciated.

Zhu Yanjun

On Fri, Jun 17, 2016 at 1:46 AM, Skidmore, Donald C <
donald.c.skidmore@intel.com> wrote:

>
>
> From: zhuyj [mailto:zyjzyj2000@gmail.com]
> Sent: Thursday, June 16, 2016 3:43 AM
> To: e1000-devel@lists.sourceforge.net; netdev <netdev@vger.kernel.org>;
> Skidmore, Donald C <donald.c.skidmore@intel.com>
> Cc: Zhu Yanjun <zyjzyj2000@gmail.com>
> Subject: Re: [PATCH 1/1] ixgbe: add fiber tranceiver plug/unplug notifier
>
> Sorry, Maybe last_tranceiver_status and tranceiver_polltime should not be
> in ixgbe_hw struct. It is better in ixgbe struct. I will change it soon.
>
> Zhu Yanjun
>
> On Wed, Jun 15, 2016 at 9:36 PM, <zyjzyj2000@gmail.com> wrote:
> From: Zhu Yanjun <zyjzyj2000@gmail.com>
>
> When the fiber tranceiver is plugged/unplugged, a netdev notifier is
>  sent. The userspace tools or kernel can receive this notifier.
>
> Signed-off-by: Zhu Yanjun <zyjzyj2000@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   24
> +++++++++++++++++++++++-
>  drivers/net/ethernet/intel/ixgbe/ixgbe_type.h |    2 ++
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 088c47c..1d8c1ff 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -5635,6 +5635,8 @@ static int ixgbe_sw_init(struct ixgbe_adapter
> *adapter)
>         hw->revision_id = pdev->revision;
>         hw->subsystem_vendor_id = pdev->subsystem_vendor;
>         hw->subsystem_device_id = pdev->subsystem_device;
> +       hw->last_tranceiver_status = IXGBE_NOT_IMPLEMENTED;
> +       hw->tranceiver_polltime = 0;
>
>         /* Set common capability flags and settings */
>         rss = min_t(int, ixgbe_max_rss_indices(adapter),
> num_online_cpus());
> @@ -7067,7 +7069,27 @@ static void ixgbe_watchdog_subtask(struct
> ixgbe_adapter *adapter)
>  static void ixgbe_sfp_detection_subtask(struct ixgbe_adapter *adapter)
>  {
>         struct ixgbe_hw *hw = &adapter->hw;
> -       s32 err;
> +       s32 err, status;
> +
> +       if ((hw->mac.ops.get_media_type(hw) == ixgbe_media_type_fiber) &&
> +           time_after(jiffies, hw->tranceiver_polltime)) {
> +               status = IXGBE_READ_REG(hw, IXGBE_ESDP) & IXGBE_ESDP_SDP2;
> +               if (status != hw->last_tranceiver_status) {
> +                       unsigned long val;
> +
> +                       if (!status) {
> +                               hw->phy.sfp_type =
> ixgbe_sfp_type_not_present;
> +                               val = NETDEV_FIBER_TRANCEIVER_UNPLUG;
> +                       } else {
> +                               val = NETDEV_FIBER_TRANCEIVER_PLUG;
> +                       }
> +                       rtnl_lock();
> +                       call_netdevice_notifiers(val, adapter->netdev);
> +                       rtnl_unlock();
> +               }
> +               hw->last_tranceiver_status = status;
> +               hw->tranceiver_polltime = jiffies + 3 * HZ;
> +       }
>
>         /* If crosstalk fix enabled verify the SFP+ cage is full */
>         if (adapter->need_crosstalk_fix) {
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> index da3d835..fe19899 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h
> @@ -3525,6 +3525,8 @@ struct ixgbe_hw {
>         bool                            force_full_reset;
>         bool                            allow_unsupported_sfp;
>         bool                            wol_enabled;
> +       s32                             last_tranceiver_status;
> +       unsigned long                   tranceiver_polltime;
>  };
>
>  struct ixgbe_info {
> --
> 1.7.9.5
>
>
> What exactly is this information (arrival/removal of a SFP+ module) needed
> for?  I can't think of any use the kernel would have, outside the driver.
> So I must assume a userspace application, if so what is it?
>
> Top that off this notification would only be actuate while the port was
> up, which limits its value even further.  The driver just doesn't bother
> keeping this information unless it is needed.   With these limitations I'm
> not seeing the advantage of adding this.
>
> Also what a given SDP is connected to different for different HW types so
> assumptions made in the patch above would be incorrect on various platforms.
>
> -Don Skidmore <donald.c.skidmore@intel.com>
>
>
>
>
>
>
>
>
>
>

[-- Attachment #2: Type: text/plain, Size: 428 bytes --]

------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity planning
reports. http://sdm.link/zohomanageengine

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

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

end of thread, other threads:[~2016-06-20  2:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-14 13:43 [PATCH net-next 1/1] ixgbe: add fiber tranceiver plug/unplug notifier zyjzyj2000
2016-06-14 13:43 ` [PATCH " zyjzyj2000
2016-06-14 13:55   ` zhuyj
2016-06-14 15:20     ` Skidmore, Donald C
2016-06-14 15:54       ` [E1000-devel] " Stephen Hemminger
2016-06-15 13:36       ` [PATCH net-next " zyjzyj2000
2016-06-15 13:36         ` [PATCH " zyjzyj2000
2016-06-16 10:43           ` zhuyj
2016-06-16 17:46             ` Skidmore, Donald C
2016-06-20  2:50               ` zhuyj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.