[dpdk-dev] net/ixgbe: support detection of hot swapped SFP/SFP+
Checks
Commit Message
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(+)
Comments
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
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
>
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
> >
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
>> >
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
> >> >
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
>> >> >
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.
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.
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.
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.
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.
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.
>
@@ -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;