| Message ID | CAE2NFgXe4uPT1WDVzSq_rr54XA7xDNuedoBgHdtd2_WekNQZ+A@mail.gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
Patrick McLean <patrickm@gaikai.com> wrote: > Gmail mangled my first message, sorry about that. Second attempt. > > In ixgbe_init_mac_link_ops_X550em, the code has a special case for > backplane media type, but does not fall through to the default case, > so the setup_fc never gets initialized. This causes a panic when it > later tries to set up the card, and the kernel dereferences the null > pointer. > > This patch lets the the function fall through, which initialized > setup_fc properly. I don't think that this is the right fix. My memory is that fc autoneg is not supported in that configuration, so setup_fc is intended to be NULL. I think it is the reference that needs to have a check added. -- Mark Rustad, Networking Division, Intel Corporation
>-----Original Message----- >From: Intel-wired-lan [mailto:intel-wired-lan-bounces@lists.osuosl.org] On >Behalf Of Rustad, Mark D >Sent: Wednesday, July 06, 2016 4:01 PM >To: Patrick McLean <patrickm@gaikai.com> >Cc: netdev <netdev@vger.kernel.org>; intel-wired-lan <intel-wired- >lan@lists.osuosl.org> >Subject: Re: [Intel-wired-lan] [PATCH] (resend) ixgbe: always initialize >setup_fc > >Patrick McLean <patrickm@gaikai.com> wrote: > >> Gmail mangled my first message, sorry about that. Second attempt. >> >> In ixgbe_init_mac_link_ops_X550em, the code has a special case for >> backplane media type, but does not fall through to the default case, >> so the setup_fc never gets initialized. This causes a panic when it >> later tries to set up the card, and the kernel dereferences the null >> pointer. >> >> This patch lets the the function fall through, which initialized >> setup_fc properly. > >I don't think that this is the right fix. My memory is that fc autoneg is setup_fc() does not configure FC autoneg and it should always be set. I posted an alternative patch that simply sets setup_fc at the beginning of the function. The fall-through in the switch statement is not a good solution because it won't work in case we need to add another case. http://patchwork.ozlabs.org/patch/646228/ Thanks, Emil
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c index 19b75cd..73e2de7 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c @@ -1653,7 +1653,6 @@ static void ixgbe_init_mac_link_ops_X550em(struct ixgbe_hw *hw) if (hw->device_id == IXGBE_DEV_ID_X550EM_A_SGMII || hw->device_id == IXGBE_DEV_ID_X550EM_A_SGMII_L) mac->ops.setup_link = ixgbe_setup_sgmii; - break; default: mac->ops.setup_fc = ixgbe_setup_fc_x550em; break;