[PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Srini J]

From: Srinivasan Jayarajan <srinidpdk@gmail.com>

    Adds support to use a different SFP/SFP+ without restarting the
    DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
    to be called on the port to detect the SFP/SFP+ change.

Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index c226e0a..85407a9 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct rte_pci_device *pci_dev)
 	status = ixgbe_pf_reset_hw(hw);
 	if (status != 0)
 		return -1;
+
+	/* Set phy type as unknown so that PHY scan is always done */
+	hw->phy.type = ixgbe_phy_unknown;
+
+	/* Identify PHY and related function pointers */
+	status = hw->phy.ops.init(hw);
+
+	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
+		PMD_INIT_LOG(ERR, "Found unsupported SFP in "
+					"ixgbe_dev_start(): %d", status);
+		return -1;
+	}
+
 	hw->mac.ops.start_hw(hw);
 	hw->mac.get_link_status = true;
 
-- 
1.8.1.4

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Lu, Wenzhuo]

Hi Srini,

> -----Original Message-----
> From: Srini J [mailto:srinidpdk@gmail.com]
> Sent: Thursday, April 20, 2017 6:48 PM
> To: Lu, Wenzhuo; Ananyev, Konstantin
> Cc: dev@dpdk.org; Srinivasan Jayarajan
> Subject: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
> 
> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
> 
>     Adds support to use a different SFP/SFP+ without restarting the
>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
>     to be called on the port to detect the SFP/SFP+ change.
> 
> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
> ---
>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index c226e0a..85407a9 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
> rte_pci_device *pci_dev)
>  	status = ixgbe_pf_reset_hw(hw);
>  	if (status != 0)
>  		return -1;
> +
> +	/* Set phy type as unknown so that PHY scan is always done */
> +	hw->phy.type = ixgbe_phy_unknown;
> +
> +	/* Identify PHY and related function pointers */
> +	status = hw->phy.ops.init(hw);
> +
> +	if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
> +		PMD_INIT_LOG(ERR, "Found unsupported SFP in "
> +					"ixgbe_dev_start(): %d", status);
> +		return -1;
> +	}
> +
I have the concern if it's a good idea to move the functions from dev_init to dev_start. Especially this function named init.
Anyway, let's listen to others opinion.


>  	hw->mac.ops.start_hw(hw);
>  	hw->mac.get_link_status = true;
> 
> --
> 1.8.1.4

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Srinivasan J]

Hi Wenzhuo,
                   I understand your concern. I had to do this change
to support hot swap of SFP/SFP+ modules without restarting the DPDK
app. If we have some other mechanism in pipeline to support hot swap
of SFP/SFP+ modules, then my changes can be ignored. I just wanted to
share the changes which worked for me back to the community.

Regards,
Srini

On Fri, Apr 21, 2017 at 10:22 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com> wrote:
> Hi Srini,
>
>> -----Original Message-----
>> From: Srini J [mailto:srinidpdk@gmail.com]
>> Sent: Thursday, April 20, 2017 6:48 PM
>> To: Lu, Wenzhuo; Ananyev, Konstantin
>> Cc: dev@dpdk.org; Srinivasan Jayarajan
>> Subject: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
>>
>> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
>>
>>     Adds support to use a different SFP/SFP+ without restarting the
>>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
>>     to be called on the port to detect the SFP/SFP+ change.
>>
>> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
>> ---
>>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index c226e0a..85407a9 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
>> rte_pci_device *pci_dev)
>>       status = ixgbe_pf_reset_hw(hw);
>>       if (status != 0)
>>               return -1;
>> +
>> +     /* Set phy type as unknown so that PHY scan is always done */
>> +     hw->phy.type = ixgbe_phy_unknown;
>> +
>> +     /* Identify PHY and related function pointers */
>> +     status = hw->phy.ops.init(hw);
>> +
>> +     if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
>> +             PMD_INIT_LOG(ERR, "Found unsupported SFP in "
>> +                                     "ixgbe_dev_start(): %d", status);
>> +             return -1;
>> +     }
>> +
> I have the concern if it's a good idea to move the functions from dev_init to dev_start. Especially this function named init.
> Anyway, let's listen to others opinion.
>
>
>>       hw->mac.ops.start_hw(hw);
>>       hw->mac.get_link_status = true;
>>
>> --
>> 1.8.1.4
>

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Lu, Wenzhuo]

Hi Srini,


> -----Original Message-----
> From: Srinivasan J [mailto:srinidpdk@gmail.com]
> Sent: Monday, April 24, 2017 2:24 PM
> To: Lu, Wenzhuo
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
> 
> Hi Wenzhuo,
>                    I understand your concern. I had to do this change to support hot
> swap of SFP/SFP+ modules without restarting the DPDK app. If we have some
> other mechanism in pipeline to support hot swap of SFP/SFP+ modules, then
> my changes can be ignored. I just wanted to share the changes which worked
> for me back to the community.
I think restarting the APP is not necessary. You can uninit the port,  then re-init the port.
Please reference http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html to see if it can help.

> 
> Regards,
> Srini
> 
> On Fri, Apr 21, 2017 at 10:22 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
> wrote:
> > Hi Srini,
> >
> >> -----Original Message-----
> >> From: Srini J [mailto:srinidpdk@gmail.com]
> >> Sent: Thursday, April 20, 2017 6:48 PM
> >> To: Lu, Wenzhuo; Ananyev, Konstantin
> >> Cc: dev@dpdk.org; Srinivasan Jayarajan
> >> Subject: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
> >>
> >> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
> >>
> >>     Adds support to use a different SFP/SFP+ without restarting the
> >>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
> >>     to be called on the port to detect the SFP/SFP+ change.
> >>
> >> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
> >> ---
> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
> >>  1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> index c226e0a..85407a9 100644
> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
> >> rte_pci_device *pci_dev)
> >>       status = ixgbe_pf_reset_hw(hw);
> >>       if (status != 0)
> >>               return -1;
> >> +
> >> +     /* Set phy type as unknown so that PHY scan is always done */
> >> +     hw->phy.type = ixgbe_phy_unknown;
> >> +
> >> +     /* Identify PHY and related function pointers */
> >> +     status = hw->phy.ops.init(hw);
> >> +
> >> +     if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
> >> +             PMD_INIT_LOG(ERR, "Found unsupported SFP in "
> >> +                                     "ixgbe_dev_start(): %d", status);
> >> +             return -1;
> >> +     }
> >> +
> > I have the concern if it's a good idea to move the functions from dev_init
> to dev_start. Especially this function named init.
> > Anyway, let's listen to others opinion.
> >
> >
> >>       hw->mac.ops.start_hw(hw);
> >>       hw->mac.get_link_status = true;
> >>
> >> --
> >> 1.8.1.4
> >

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Srinivasan J]

Hi Wenzhuo,
                    Port hot-plug would require the DPDK app to
dynamically mange ports and the kernel to support PCI hot-plugging.
When multiple ports are detached/attached (SFP's plugged in/out),
would the DPDK port id's remain same? If not, app needs to handle port
id change for a give NIC.

While port hot-plug can be used to dynamically handle SFP
hot-plugging, I feel it's not the optimal solution.

Regards,
Srini

On Mon, Apr 24, 2017 at 12:45 PM, Lu, Wenzhuo <wenzhuo.lu@intel.com> wrote:
> Hi Srini,
>
>
>> -----Original Message-----
>> From: Srinivasan J [mailto:srinidpdk@gmail.com]
>> Sent: Monday, April 24, 2017 2:24 PM
>> To: Lu, Wenzhuo
>> Cc: Ananyev, Konstantin; dev@dpdk.org
>> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
>>
>> Hi Wenzhuo,
>>                    I understand your concern. I had to do this change to support hot
>> swap of SFP/SFP+ modules without restarting the DPDK app. If we have some
>> other mechanism in pipeline to support hot swap of SFP/SFP+ modules, then
>> my changes can be ignored. I just wanted to share the changes which worked
>> for me back to the community.
> I think restarting the APP is not necessary. You can uninit the port,  then re-init the port.
> Please reference http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html to see if it can help.
>
>>
>> Regards,
>> Srini
>>
>> On Fri, Apr 21, 2017 at 10:22 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> wrote:
>> > Hi Srini,
>> >
>> >> -----Original Message-----
>> >> From: Srini J [mailto:srinidpdk@gmail.com]
>> >> Sent: Thursday, April 20, 2017 6:48 PM
>> >> To: Lu, Wenzhuo; Ananyev, Konstantin
>> >> Cc: dev@dpdk.org; Srinivasan Jayarajan
>> >> Subject: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
>> >>
>> >> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
>> >>
>> >>     Adds support to use a different SFP/SFP+ without restarting the
>> >>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
>> >>     to be called on the port to detect the SFP/SFP+ change.
>> >>
>> >> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
>> >> ---
>> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
>> >>  1 file changed, 13 insertions(+)
>> >>
>> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> index c226e0a..85407a9 100644
>> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
>> >> rte_pci_device *pci_dev)
>> >>       status = ixgbe_pf_reset_hw(hw);
>> >>       if (status != 0)
>> >>               return -1;
>> >> +
>> >> +     /* Set phy type as unknown so that PHY scan is always done */
>> >> +     hw->phy.type = ixgbe_phy_unknown;
>> >> +
>> >> +     /* Identify PHY and related function pointers */
>> >> +     status = hw->phy.ops.init(hw);
>> >> +
>> >> +     if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
>> >> +             PMD_INIT_LOG(ERR, "Found unsupported SFP in "
>> >> +                                     "ixgbe_dev_start(): %d", status);
>> >> +             return -1;
>> >> +     }
>> >> +
>> > I have the concern if it's a good idea to move the functions from dev_init
>> to dev_start. Especially this function named init.
>> > Anyway, let's listen to others opinion.
>> >
>> >
>> >>       hw->mac.ops.start_hw(hw);
>> >>       hw->mac.get_link_status = true;
>> >>
>> >> --
>> >> 1.8.1.4
>> >

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Lu, Wenzhuo]

Hi Srini,


> -----Original Message-----
> From: Srinivasan J [mailto:srinidpdk@gmail.com]
> Sent: Monday, April 24, 2017 6:51 PM
> To: Lu, Wenzhuo
> Cc: Ananyev, Konstantin; dev@dpdk.org
> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
> 
> Hi Wenzhuo,
>                     Port hot-plug would require the DPDK app to dynamically mange
> ports and the kernel to support PCI hot-plugging.
> When multiple ports are detached/attached (SFP's plugged in/out), would
> the DPDK port id's remain same? If not, app needs to handle port id change
> for a give NIC.
Yes, APP needs to handle the port id change. Agree this patch is a much simpler solution than the port hot-plug.
So I don’t object it :)

> 
> While port hot-plug can be used to dynamically handle SFP hot-plugging, I
> feel it's not the optimal solution.
> 
> Regards,
> Srini
> 
> On Mon, Apr 24, 2017 at 12:45 PM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
> wrote:
> > Hi Srini,
> >
> >
> >> -----Original Message-----
> >> From: Srinivasan J [mailto:srinidpdk@gmail.com]
> >> Sent: Monday, April 24, 2017 2:24 PM
> >> To: Lu, Wenzhuo
> >> Cc: Ananyev, Konstantin; dev@dpdk.org
> >> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped
> >> SFP/SFP+
> >>
> >> Hi Wenzhuo,
> >>                    I understand your concern. I had to do this change
> >> to support hot swap of SFP/SFP+ modules without restarting the DPDK
> >> app. If we have some other mechanism in pipeline to support hot swap
> >> of SFP/SFP+ modules, then my changes can be ignored. I just wanted to
> >> share the changes which worked for me back to the community.
> > I think restarting the APP is not necessary. You can uninit the port,  then
> re-init the port.
> > Please reference
> http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html to
> see if it can help.
> >
> >>
> >> Regards,
> >> Srini
> >>
> >> On Fri, Apr 21, 2017 at 10:22 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
> >> wrote:
> >> > Hi Srini,
> >> >
> >> >> -----Original Message-----
> >> >> From: Srini J [mailto:srinidpdk@gmail.com]
> >> >> Sent: Thursday, April 20, 2017 6:48 PM
> >> >> To: Lu, Wenzhuo; Ananyev, Konstantin
> >> >> Cc: dev@dpdk.org; Srinivasan Jayarajan
> >> >> Subject: [PATCH] net/ixgbe: support detection of hot swapped
> >> >> SFP/SFP+
> >> >>
> >> >> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
> >> >>
> >> >>     Adds support to use a different SFP/SFP+ without restarting the
> >> >>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
> >> >>     to be called on the port to detect the SFP/SFP+ change.
> >> >>
> >> >> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
> >> >> ---
> >> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
> >> >>  1 file changed, 13 insertions(+)
> >> >>
> >> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> >> index c226e0a..85407a9 100644
> >> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >> >> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
> >> >> rte_pci_device *pci_dev)
> >> >>       status = ixgbe_pf_reset_hw(hw);
> >> >>       if (status != 0)
> >> >>               return -1;
> >> >> +
> >> >> +     /* Set phy type as unknown so that PHY scan is always done */
> >> >> +     hw->phy.type = ixgbe_phy_unknown;
> >> >> +
> >> >> +     /* Identify PHY and related function pointers */
> >> >> +     status = hw->phy.ops.init(hw);
> >> >> +
> >> >> +     if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
> >> >> +             PMD_INIT_LOG(ERR, "Found unsupported SFP in "
> >> >> +                                     "ixgbe_dev_start(): %d", status);
> >> >> +             return -1;
> >> >> +     }
> >> >> +
> >> > I have the concern if it's a good idea to move the functions from
> >> > dev_init
> >> to dev_start. Especially this function named init.
> >> > Anyway, let's listen to others opinion.
> >> >
> >> >
> >> >>       hw->mac.ops.start_hw(hw);
> >> >>       hw->mac.get_link_status = true;
> >> >>
> >> >> --
> >> >> 1.8.1.4
> >> >

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Srinivasan J]

Hi,
                   Do we need an explicit "Acked-by" keyword for this
patch to be accepted and applied?

Thanks,
Srini

On Tue, Apr 25, 2017 at 6:53 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com> wrote:
> Hi Srini,
>
>
>> -----Original Message-----
>> From: Srinivasan J [mailto:srinidpdk@gmail.com]
>> Sent: Monday, April 24, 2017 6:51 PM
>> To: Lu, Wenzhuo
>> Cc: Ananyev, Konstantin; dev@dpdk.org
>> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+
>>
>> Hi Wenzhuo,
>>                     Port hot-plug would require the DPDK app to dynamically mange
>> ports and the kernel to support PCI hot-plugging.
>> When multiple ports are detached/attached (SFP's plugged in/out), would
>> the DPDK port id's remain same? If not, app needs to handle port id change
>> for a give NIC.
> Yes, APP needs to handle the port id change. Agree this patch is a much simpler solution than the port hot-plug.
> So I don’t object it :)
>
>>
>> While port hot-plug can be used to dynamically handle SFP hot-plugging, I
>> feel it's not the optimal solution.
>>
>> Regards,
>> Srini
>>
>> On Mon, Apr 24, 2017 at 12:45 PM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> wrote:
>> > Hi Srini,
>> >
>> >
>> >> -----Original Message-----
>> >> From: Srinivasan J [mailto:srinidpdk@gmail.com]
>> >> Sent: Monday, April 24, 2017 2:24 PM
>> >> To: Lu, Wenzhuo
>> >> Cc: Ananyev, Konstantin; dev@dpdk.org
>> >> Subject: Re: [PATCH] net/ixgbe: support detection of hot swapped
>> >> SFP/SFP+
>> >>
>> >> Hi Wenzhuo,
>> >>                    I understand your concern. I had to do this change
>> >> to support hot swap of SFP/SFP+ modules without restarting the DPDK
>> >> app. If we have some other mechanism in pipeline to support hot swap
>> >> of SFP/SFP+ modules, then my changes can be ignored. I just wanted to
>> >> share the changes which worked for me back to the community.
>> > I think restarting the APP is not necessary. You can uninit the port,  then
>> re-init the port.
>> > Please reference
>> http://dpdk.org/doc/guides/prog_guide/port_hotplug_framework.html to
>> see if it can help.
>> >
>> >>
>> >> Regards,
>> >> Srini
>> >>
>> >> On Fri, Apr 21, 2017 at 10:22 AM, Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> >> wrote:
>> >> > Hi Srini,
>> >> >
>> >> >> -----Original Message-----
>> >> >> From: Srini J [mailto:srinidpdk@gmail.com]
>> >> >> Sent: Thursday, April 20, 2017 6:48 PM
>> >> >> To: Lu, Wenzhuo; Ananyev, Konstantin
>> >> >> Cc: dev@dpdk.org; Srinivasan Jayarajan
>> >> >> Subject: [PATCH] net/ixgbe: support detection of hot swapped
>> >> >> SFP/SFP+
>> >> >>
>> >> >> From: Srinivasan Jayarajan <srinidpdk@gmail.com>
>> >> >>
>> >> >>     Adds support to use a different SFP/SFP+ without restarting the
>> >> >>     DPDK app. rte_eth_dev_stop()/rte_eth_dev_start() will need
>> >> >>     to be called on the port to detect the SFP/SFP+ change.
>> >> >>
>> >> >> Signed-off-by: Srinivasan Jayarajan <srinidpdk@gmail.com>
>> >> >> ---
>> >> >>  drivers/net/ixgbe/ixgbe_ethdev.c | 13 +++++++++++++
>> >> >>  1 file changed, 13 insertions(+)
>> >> >>
>> >> >> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> >> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> >> index c226e0a..85407a9 100644
>> >> >> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> >> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >> >> @@ -2520,6 +2520,19 @@ static int eth_ixgbevf_pci_remove(struct
>> >> >> rte_pci_device *pci_dev)
>> >> >>       status = ixgbe_pf_reset_hw(hw);
>> >> >>       if (status != 0)
>> >> >>               return -1;
>> >> >> +
>> >> >> +     /* Set phy type as unknown so that PHY scan is always done */
>> >> >> +     hw->phy.type = ixgbe_phy_unknown;
>> >> >> +
>> >> >> +     /* Identify PHY and related function pointers */
>> >> >> +     status = hw->phy.ops.init(hw);
>> >> >> +
>> >> >> +     if (status == IXGBE_ERR_SFP_NOT_SUPPORTED) {
>> >> >> +             PMD_INIT_LOG(ERR, "Found unsupported SFP in "
>> >> >> +                                     "ixgbe_dev_start(): %d", status);
>> >> >> +             return -1;
>> >> >> +     }
>> >> >> +
>> >> > I have the concern if it's a good idea to move the functions from
>> >> > dev_init
>> >> to dev_start. Especially this function named init.
>> >> > Anyway, let's listen to others opinion.
>> >> >
>> >> >
>> >> >>       hw->mac.ops.start_hw(hw);
>> >> >>       hw->mac.get_link_status = true;
>> >> >>
>> >> >> --
>> >> >> 1.8.1.4
>> >> >

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Thomas Monjalon]

06/05/2017 15:51, Srinivasan J:
> Hi,
>                    Do we need an explicit "Acked-by" keyword for this
> patch to be accepted and applied?

Yes, given it is not a trivial patch, an ack from the maintainer is required.
Anyway, it has been submitted too late for 17.05 testing.

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Dai, Wei]

Hi, Srini

There is a bit confusion. Your patch shows that your code is added into the function eth_ixgbevf_pci_remove( ).
But it is not. It is added into the fucntion ixgbe_dev_start( ), right ?
So would you please rebase it to R 17.05 ?

Which type of ixgbe device id did you tested ?

There are many MAC types with different device id.

The function ixgbe_pf_reset_hw(hw) is called before your adding code.
ixgbe_pf_reset_hw() calls hw->mac.ops.reset_hw( ) which may points to following different function for different MAC type.
Ixgbe_reset_hw_82598( ) calls hw->phy.ops.init(hw) if hw->phy.reset_disable == false .
Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally.
ixgbe_reset_hw_X540( ) doesn't' call pw->phy.ops.init(hw). For X540,  hw->phy.ops.init points to ixgbe_init_phy_ops_generic() which only initialize some function pointers.
Ixgbe_rest_hw_x550em() calls hw->phy.ops.init(hw) unconditionally.

And for VF,  ixgbe_reset_hw_vf( ) and ixgbevf_hv_reset_hw_vf( ) don't call hw->phy.ops.init(hw) anywhere.

Thanks & Best Regards
-Wei

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Sunday, May 7, 2017 6:36 AM
> To: Srinivasan J <srinidpdk@gmail.com>
> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
> Konstantin <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot swapped
> SFP/SFP+
> 
> 06/05/2017 15:51, Srinivasan J:
> > Hi,
> >                    Do we need an explicit "Acked-by" keyword for this
> > patch to be accepted and applied?
> 
> Yes, given it is not a trivial patch, an ack from the maintainer is required.
> Anyway, it has been submitted too late for 17.05 testing.

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Srinivasan J]

Hi Wei,
          Yes the changes are in ixgbe_dev_start( ),  the patch shows
the function as eth_ixgbevf_pci_remove() probably due to the way diff
recognizes the change. I have tested the change using Intel
Corporation 82599ES.

Thanks,
Srini

On Tue, May 16, 2017 at 9:04 AM, Dai, Wei <wei.dai@intel.com> wrote:
> Hi, Srini
>
> There is a bit confusion. Your patch shows that your code is added into the function eth_ixgbevf_pci_remove( ).
> But it is not. It is added into the fucntion ixgbe_dev_start( ), right ?
> So would you please rebase it to R 17.05 ?
>
> Which type of ixgbe device id did you tested ?
>
> There are many MAC types with different device id.
>
> The function ixgbe_pf_reset_hw(hw) is called before your adding code.
> ixgbe_pf_reset_hw() calls hw->mac.ops.reset_hw( ) which may points to following different function for different MAC type.
> Ixgbe_reset_hw_82598( ) calls hw->phy.ops.init(hw) if hw->phy.reset_disable == false .
> Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally.
> ixgbe_reset_hw_X540( ) doesn't' call pw->phy.ops.init(hw). For X540,  hw->phy.ops.init points to ixgbe_init_phy_ops_generic() which only initialize some function pointers.
> Ixgbe_rest_hw_x550em() calls hw->phy.ops.init(hw) unconditionally.
>
> And for VF,  ixgbe_reset_hw_vf( ) and ixgbevf_hv_reset_hw_vf( ) don't call hw->phy.ops.init(hw) anywhere.
>
> Thanks & Best Regards
> -Wei
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>> Sent: Sunday, May 7, 2017 6:36 AM
>> To: Srinivasan J <srinidpdk@gmail.com>
>> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
>> Konstantin <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot swapped
>> SFP/SFP+
>>
>> 06/05/2017 15:51, Srinivasan J:
>> > Hi,
>> >                    Do we need an explicit "Acked-by" keyword for this
>> > patch to be accepted and applied?
>>
>> Yes, given it is not a trivial patch, an ack from the maintainer is required.
>> Anyway, it has been submitted too late for 17.05 testing.

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Ferruh Yigit]

On 5/19/2017 11:04 AM, Srinivasan J wrote:
> Hi Wei,
>           Yes the changes are in ixgbe_dev_start( ),  the patch shows
> the function as eth_ixgbevf_pci_remove() probably due to the way diff
> recognizes the change. I have tested the change using Intel
> Corporation 82599ES.

Hi Srinivasan, Wei,

What is the latest status of the patch? Are all issues pointed by Wie
addressed in the patch, or are we waiting for a new version?

Thanks,
ferruh

> 
> Thanks,
> Srini
> 
> On Tue, May 16, 2017 at 9:04 AM, Dai, Wei <wei.dai@intel.com> wrote:
>> Hi, Srini
>>
>> There is a bit confusion. Your patch shows that your code is added into the function eth_ixgbevf_pci_remove( ).
>> But it is not. It is added into the fucntion ixgbe_dev_start( ), right ?
>> So would you please rebase it to R 17.05 ?
>>
>> Which type of ixgbe device id did you tested ?
>>
>> There are many MAC types with different device id.
>>
>> The function ixgbe_pf_reset_hw(hw) is called before your adding code.
>> ixgbe_pf_reset_hw() calls hw->mac.ops.reset_hw( ) which may points to following different function for different MAC type.
>> Ixgbe_reset_hw_82598( ) calls hw->phy.ops.init(hw) if hw->phy.reset_disable == false .
>> Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally.
>> ixgbe_reset_hw_X540( ) doesn't' call pw->phy.ops.init(hw). For X540,  hw->phy.ops.init points to ixgbe_init_phy_ops_generic() which only initialize some function pointers.
>> Ixgbe_rest_hw_x550em() calls hw->phy.ops.init(hw) unconditionally.
>>
>> And for VF,  ixgbe_reset_hw_vf( ) and ixgbevf_hv_reset_hw_vf( ) don't call hw->phy.ops.init(hw) anywhere.
>>
>> Thanks & Best Regards
>> -Wei
>>
>>> -----Original Message-----
>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
>>> Sent: Sunday, May 7, 2017 6:36 AM
>>> To: Srinivasan J <srinidpdk@gmail.com>
>>> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
>>> Konstantin <konstantin.ananyev@intel.com>
>>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot swapped
>>> SFP/SFP+
>>>
>>> 06/05/2017 15:51, Srinivasan J:
>>>> Hi,
>>>>                    Do we need an explicit "Acked-by" keyword for this
>>>> patch to be accepted and applied?
>>>
>>> Yes, given it is not a trivial patch, an ack from the maintainer is required.
>>> Anyway, it has been submitted too late for 17.05 testing.

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Dai, Wei]

HI, Srini

Sorry for late response.

As I have pointed out that Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally,
I think it is no need to call hw->phy.ops.init(hw) after ixgbe_pf_reset_hw(hw) at least for 82599.
I also think that only moving "hw->phy.type = ixgbe_phy_unknown" just before ixgbe_pf_reset_hw(hw) is OK.

What's more, how about X540 and X550 ?
I have just got a X540 and a X550 NIC with copper interface, so I only can plug in/out the RJ45 line to help test it.

Is your patch designed for plugging out original SFP and then plugging in another different type of SFP ?

By the way, I'd like you provide more details on how to test your patch? With testpmd ? Or other app ?

Thanks
-Wei

> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Thursday, June 29, 2017 9:00 PM
> To: Srinivasan J <srinidpdk@gmail.com>; Dai, Wei <wei.dai@intel.com>
> Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Lu,
> Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot
> swapped SFP/SFP+
> 
> On 5/19/2017 11:04 AM, Srinivasan J wrote:
> > Hi Wei,
> >           Yes the changes are in ixgbe_dev_start( ),  the patch shows
> > the function as eth_ixgbevf_pci_remove() probably due to the way diff
> > recognizes the change. I have tested the change using Intel
> > Corporation 82599ES.
> 
> Hi Srinivasan, Wei,
> 
> What is the latest status of the patch? Are all issues pointed by Wie
> addressed in the patch, or are we waiting for a new version?
> 
> Thanks,
> ferruh
> 
> >
> > Thanks,
> > Srini
> >
> > On Tue, May 16, 2017 at 9:04 AM, Dai, Wei <wei.dai@intel.com> wrote:
> >> Hi, Srini
> >>
> >> There is a bit confusion. Your patch shows that your code is added into
> the function eth_ixgbevf_pci_remove( ).
> >> But it is not. It is added into the fucntion ixgbe_dev_start( ), right ?
> >> So would you please rebase it to R 17.05 ?
> >>
> >> Which type of ixgbe device id did you tested ?
> >>
> >> There are many MAC types with different device id.
> >>
> >> The function ixgbe_pf_reset_hw(hw) is called before your adding code.
> >> ixgbe_pf_reset_hw() calls hw->mac.ops.reset_hw( ) which may points to
> following different function for different MAC type.
> >> Ixgbe_reset_hw_82598( ) calls hw->phy.ops.init(hw) if
> hw->phy.reset_disable == false .
> >> Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally.
> >> ixgbe_reset_hw_X540( ) doesn't' call pw->phy.ops.init(hw). For X540,
> hw->phy.ops.init points to ixgbe_init_phy_ops_generic() which only initialize
> some function pointers.
> >> Ixgbe_rest_hw_x550em() calls hw->phy.ops.init(hw) unconditionally.
> >>
> >> And for VF,  ixgbe_reset_hw_vf( ) and ixgbevf_hv_reset_hw_vf( ) don't
> call hw->phy.ops.init(hw) anywhere.
> >>
> >> Thanks & Best Regards
> >> -Wei
> >>
> >>> -----Original Message-----
> >>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> Monjalon
> >>> Sent: Sunday, May 7, 2017 6:36 AM
> >>> To: Srinivasan J <srinidpdk@gmail.com>
> >>> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
> >>> Konstantin <konstantin.ananyev@intel.com>
> >>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot
> >>> swapped SFP/SFP+
> >>>
> >>> 06/05/2017 15:51, Srinivasan J:
> >>>> Hi,
> >>>>                    Do we need an explicit "Acked-by" keyword for
> >>>> this patch to be accepted and applied?
> >>>
> >>> Yes, given it is not a trivial patch, an ack from the maintainer is required.
> >>> Anyway, it has been submitted too late for 17.05 testing.

Re: [PATCH] net/ixgbe: support detection of hot swapped SFP/SFP+

[Ferruh Yigit]

On 7/18/2017 3:56 PM, Dai, Wei wrote:
> HI, Srini
> 
> Sorry for late response.
> 
> As I have pointed out that Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally,
> I think it is no need to call hw->phy.ops.init(hw) after ixgbe_pf_reset_hw(hw) at least for 82599.
> I also think that only moving "hw->phy.type = ixgbe_phy_unknown" just before ixgbe_pf_reset_hw(hw) is OK.
> 
> What's more, how about X540 and X550 ?
> I have just got a X540 and a X550 NIC with copper interface, so I only can plug in/out the RJ45 line to help test it.
> 
> Is your patch designed for plugging out original SFP and then plugging in another different type of SFP ?
> 
> By the way, I'd like you provide more details on how to test your patch? With testpmd ? Or other app ?

This is an old patch, with no update for a while.

If this is still needed please shout, otherwise patch will be removed.

Thanks,
ferruh


> 
> Thanks
> -Wei
> 
>> -----Original Message-----
>> From: Yigit, Ferruh
>> Sent: Thursday, June 29, 2017 9:00 PM
>> To: Srinivasan J <srinidpdk@gmail.com>; Dai, Wei <wei.dai@intel.com>
>> Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Lu,
>> Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot
>> swapped SFP/SFP+
>>
>> On 5/19/2017 11:04 AM, Srinivasan J wrote:
>>> Hi Wei,
>>>           Yes the changes are in ixgbe_dev_start( ),  the patch shows
>>> the function as eth_ixgbevf_pci_remove() probably due to the way diff
>>> recognizes the change. I have tested the change using Intel
>>> Corporation 82599ES.
>>
>> Hi Srinivasan, Wei,
>>
>> What is the latest status of the patch? Are all issues pointed by Wie
>> addressed in the patch, or are we waiting for a new version?
>>
>> Thanks,
>> ferruh
>>
>>>
>>> Thanks,
>>> Srini
>>>
>>> On Tue, May 16, 2017 at 9:04 AM, Dai, Wei <wei.dai@intel.com> wrote:
>>>> Hi, Srini
>>>>
>>>> There is a bit confusion. Your patch shows that your code is added into
>> the function eth_ixgbevf_pci_remove( ).
>>>> But it is not. It is added into the fucntion ixgbe_dev_start( ), right ?
>>>> So would you please rebase it to R 17.05 ?
>>>>
>>>> Which type of ixgbe device id did you tested ?
>>>>
>>>> There are many MAC types with different device id.
>>>>
>>>> The function ixgbe_pf_reset_hw(hw) is called before your adding code.
>>>> ixgbe_pf_reset_hw() calls hw->mac.ops.reset_hw( ) which may points to
>> following different function for different MAC type.
>>>> Ixgbe_reset_hw_82598( ) calls hw->phy.ops.init(hw) if
>> hw->phy.reset_disable == false .
>>>> Ixgbe_reset_hw_82599( ) calls hw->phy.ops.init(hw) unconditionally.
>>>> ixgbe_reset_hw_X540( ) doesn't' call pw->phy.ops.init(hw). For X540,
>> hw->phy.ops.init points to ixgbe_init_phy_ops_generic() which only initialize
>> some function pointers.
>>>> Ixgbe_rest_hw_x550em() calls hw->phy.ops.init(hw) unconditionally.
>>>>
>>>> And for VF,  ixgbe_reset_hw_vf( ) and ixgbevf_hv_reset_hw_vf( ) don't
>> call hw->phy.ops.init(hw) anywhere.
>>>>
>>>> Thanks & Best Regards
>>>> -Wei
>>>>
>>>>> -----Original Message-----
>>>>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
>> Monjalon
>>>>> Sent: Sunday, May 7, 2017 6:36 AM
>>>>> To: Srinivasan J <srinidpdk@gmail.com>
>>>>> Cc: dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev,
>>>>> Konstantin <konstantin.ananyev@intel.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: support detection of hot
>>>>> swapped SFP/SFP+
>>>>>
>>>>> 06/05/2017 15:51, Srinivasan J:
>>>>>> Hi,
>>>>>>                    Do we need an explicit "Acked-by" keyword for
>>>>>> this patch to be accepted and applied?
>>>>>
>>>>> Yes, given it is not a trivial patch, an ack from the maintainer is required.
>>>>> Anyway, it has been submitted too late for 17.05 testing.
>