* [PATCH net-next 3/5] net: phy: mscc: remove unneeded parenthesis
From: Quentin Schulz @ 2018-09-14 8:33 UTC (permalink / raw)
To: davem, andrew, f.fainelli
Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni,
Quentin Schulz
In-Reply-To: <cover.616d15610d44a0e3d463acd8119859f243163ad2.1536913944.git-series.quentin.schulz@bootlin.com>
The == operator precedes the || operator, so we can remove the
parenthesis around (a == b) || (c == d).
The condition is rather explicit and short so removing the parenthesis
definitely does not make it harder to read.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
drivers/net/phy/mscc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index c0a9ea9..734d9fb 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -301,7 +301,7 @@ static int vsc85xx_mdix_set(struct phy_device *phydev, u8 mdix)
u16 reg_val;
reg_val = phy_read(phydev, MSCC_PHY_BYPASS_CONTROL);
- if ((mdix == ETH_TP_MDI) || (mdix == ETH_TP_MDI_X)) {
+ if (mdix == ETH_TP_MDI || mdix == ETH_TP_MDI_X) {
reg_val |= (DISABLE_PAIR_SWAP_CORR_MASK |
DISABLE_POLARITY_CORR_MASK |
DISABLE_HP_AUTO_MDIX_MASK);
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH net-next 4/5] net: phy: mscc: shorten `x != 0` condition to `x`
From: Quentin Schulz @ 2018-09-14 8:33 UTC (permalink / raw)
To: davem, andrew, f.fainelli
Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni,
Quentin Schulz
In-Reply-To: <cover.616d15610d44a0e3d463acd8119859f243163ad2.1536913944.git-series.quentin.schulz@bootlin.com>
`if (x != 0)` is basically a more verbose version of `if (x)` so let's
use the latter so it's consistent throughout the whole driver.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
drivers/net/phy/mscc.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 734d9fb..efa9352 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -311,11 +311,11 @@ static int vsc85xx_mdix_set(struct phy_device *phydev, u8 mdix)
DISABLE_HP_AUTO_MDIX_MASK);
}
rc = phy_write(phydev, MSCC_PHY_BYPASS_CONTROL, reg_val);
- if (rc != 0)
+ if (rc)
return rc;
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
- if (rc != 0)
+ if (rc)
return rc;
reg_val = phy_read(phydev, MSCC_PHY_EXT_MODE_CNTL);
@@ -325,11 +325,11 @@ static int vsc85xx_mdix_set(struct phy_device *phydev, u8 mdix)
else if (mdix == ETH_TP_MDI_X)
reg_val |= FORCE_MDI_CROSSOVER_MDIX;
rc = phy_write(phydev, MSCC_PHY_EXT_MODE_CNTL, reg_val);
- if (rc != 0)
+ if (rc)
return rc;
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
- if (rc != 0)
+ if (rc)
return rc;
return genphy_restart_aneg(phydev);
@@ -341,7 +341,7 @@ static int vsc85xx_downshift_get(struct phy_device *phydev, u8 *count)
u16 reg_val;
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
- if (rc != 0)
+ if (rc)
goto out;
reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
@@ -373,14 +373,14 @@ static int vsc85xx_downshift_set(struct phy_device *phydev, u8 count)
}
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED);
- if (rc != 0)
+ if (rc)
goto out;
reg_val = phy_read(phydev, MSCC_PHY_ACTIPHY_CNTL);
reg_val &= ~(DOWNSHIFT_CNTL_MASK);
reg_val |= count;
rc = phy_write(phydev, MSCC_PHY_ACTIPHY_CNTL, reg_val);
- if (rc != 0)
+ if (rc)
goto out;
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
@@ -401,7 +401,7 @@ static int vsc85xx_wol_set(struct phy_device *phydev,
mutex_lock(&phydev->lock);
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
- if (rc != 0)
+ if (rc)
goto out_unlock;
if (wol->wolopts & WAKE_MAGIC) {
@@ -439,7 +439,7 @@ static int vsc85xx_wol_set(struct phy_device *phydev,
phy_write(phydev, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
- if (rc != 0)
+ if (rc)
goto out_unlock;
if (wol->wolopts & WAKE_MAGIC) {
@@ -447,14 +447,14 @@ static int vsc85xx_wol_set(struct phy_device *phydev,
reg_val = phy_read(phydev, MII_VSC85XX_INT_MASK);
reg_val |= MII_VSC85XX_INT_MASK_WOL;
rc = phy_write(phydev, MII_VSC85XX_INT_MASK, reg_val);
- if (rc != 0)
+ if (rc)
goto out_unlock;
} else {
/* Disable the WOL interrupt */
reg_val = phy_read(phydev, MII_VSC85XX_INT_MASK);
reg_val &= (~MII_VSC85XX_INT_MASK_WOL);
rc = phy_write(phydev, MII_VSC85XX_INT_MASK, reg_val);
- if (rc != 0)
+ if (rc)
goto out_unlock;
}
/* Clear WOL iterrupt status */
@@ -595,13 +595,13 @@ static int vsc85xx_edge_rate_cntl_set(struct phy_device *phydev, u8 edge_rate)
mutex_lock(&phydev->lock);
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
- if (rc != 0)
+ if (rc)
goto out_unlock;
reg_val = phy_read(phydev, MSCC_PHY_WOL_MAC_CONTROL);
reg_val &= ~(EDGE_RATE_CNTL_MASK);
reg_val |= (edge_rate << EDGE_RATE_CNTL_POS);
rc = phy_write(phydev, MSCC_PHY_WOL_MAC_CONTROL, reg_val);
- if (rc != 0)
+ if (rc)
goto out_unlock;
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_STANDARD);
@@ -636,7 +636,7 @@ static int vsc85xx_mac_if_set(struct phy_device *phydev,
goto out_unlock;
}
rc = phy_write(phydev, MSCC_PHY_EXT_PHY_CNTL_1, reg_val);
- if (rc != 0)
+ if (rc)
goto out_unlock;
rc = genphy_soft_reset(phydev);
@@ -655,7 +655,7 @@ static int vsc85xx_default_config(struct phy_device *phydev)
phydev->mdix_ctrl = ETH_TP_MDI_AUTO;
mutex_lock(&phydev->lock);
rc = vsc85xx_phy_page_set(phydev, MSCC_PHY_PAGE_EXTENDED_2);
- if (rc != 0)
+ if (rc)
goto out_unlock;
reg_val = phy_read(phydev, MSCC_PHY_RGMII_CNTL);
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH net-next 5/5] net: phy: mscc: remove unneeded temporary variable
From: Quentin Schulz @ 2018-09-14 8:33 UTC (permalink / raw)
To: davem, andrew, f.fainelli
Cc: allan.nielsen, linux-kernel, netdev, thomas.petazzoni,
Quentin Schulz
In-Reply-To: <cover.616d15610d44a0e3d463acd8119859f243163ad2.1536913944.git-series.quentin.schulz@bootlin.com>
Here, the rc variable is either used only for the condition right after
the assignment or right before being used as the return value of the
function it's being used in.
So let's remove this unneeded temporary variable whenever possible.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
drivers/net/phy/mscc.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index efa9352..24f4754 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -199,10 +199,7 @@ static const struct vsc8531_edge_rate_table edge_table[] = {
static int vsc85xx_phy_page_set(struct phy_device *phydev, u16 page)
{
- int rc;
-
- rc = phy_write(phydev, MSCC_EXT_PAGE_ACCESS, page);
- return rc;
+ return phy_write(phydev, MSCC_EXT_PAGE_ACCESS, page);
}
static int vsc85xx_get_sset_count(struct phy_device *phydev)
@@ -504,7 +501,7 @@ static void vsc85xx_wol_get(struct phy_device *phydev,
static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
{
u32 vdd, sd;
- int rc, i, j;
+ int i, j;
struct device *dev = &phydev->mdio.dev;
struct device_node *of_node = dev->of_node;
u8 sd_array_size = ARRAY_SIZE(edge_table[0].slowdown);
@@ -512,12 +509,10 @@ static int vsc85xx_edge_rate_magic_get(struct phy_device *phydev)
if (!of_node)
return -ENODEV;
- rc = of_property_read_u32(of_node, "vsc8531,vddmac", &vdd);
- if (rc != 0)
+ if (of_property_read_u32(of_node, "vsc8531,vddmac", &vdd))
vdd = MSCC_VDDMAC_3300;
- rc = of_property_read_u32(of_node, "vsc8531,edge-slowdown", &sd);
- if (rc != 0)
+ if (of_property_read_u32(of_node, "vsc8531,edge-slowdown", &sd))
sd = 0;
for (i = 0; i < ARRAY_SIZE(edge_table); i++)
@@ -762,9 +757,7 @@ static int vsc85xx_config_init(struct phy_device *phydev)
return rc;
}
- rc = genphy_config_init(phydev);
-
- return rc;
+ return genphy_config_init(phydev);
}
static int vsc85xx_ack_interrupt(struct phy_device *phydev)
--
git-series 0.9.1
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Jason Wang @ 2018-09-14 3:27 UTC (permalink / raw)
To: Willem de Bruijn
Cc: f.fainelli, Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <CAF=yD-L1h=EWF8DgAZsUCbb58tZhh5vDkPTRdwaZ895==OxTxA@mail.gmail.com>
On 2018年09月13日 22:58, Willem de Bruijn wrote:
> On Thu, Sep 13, 2018 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>> On 2018年09月13日 07:27, Willem de Bruijn wrote:
>>> On Wed, Sep 12, 2018 at 3:11 PM Willem de Bruijn
>>> <willemdebruijn.kernel@gmail.com> wrote:
>>>> On Wed, Sep 12, 2018 at 2:16 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>
>>>>> On 9/12/2018 11:07 AM, Willem de Bruijn wrote:
>>>>>> On Wed, Sep 12, 2018 at 1:42 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>>>
>>>>>>> On 9/9/2018 3:44 PM, Willem de Bruijn wrote:
>>>>>>>> From: Willem de Bruijn <willemb@google.com>
>>>>>>>>
>>>>>>>> Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
>>>>>>>> Interrupt moderation is currently not supported, so these accept and
>>>>>>>> display the default settings of 0 usec and 1 frame.
>>>>>>>>
>>>>>>>> Toggle tx napi through a bit in tx-frames. So as to not interfere
>>>>>>>> with possible future interrupt moderation, use bit 10, well outside
>>>>>>>> the reasonable range of real interrupt moderation values.
>>>>>>>>
>>>>>>>> Changes are not atomic. The tx IRQ, napi BH and transmit path must
>>>>>>>> be quiesced when switching modes. Only allow changing this setting
>>>>>>>> when the device is down.
>>>>>>> Humm, would not a private ethtool flag to switch TX NAPI on/off be more
>>>>>>> appropriate rather than use the coalescing configuration API here?
>>>>>> What do you mean by private ethtool flag? A new field in ethtool
>>>>>> --features (-k)?
>>>>> I meant using ethtool_drvinfo::n_priv_flags, ETH_SS_PRIV_FLAGS and then
>>>>> ETHTOOL_GFPFLAGS and ETHTOOL_SPFLAGS to control the toggling of that
>>>>> private flag. mlx5 has a number of privates flags for instance.
>>>> Interesting, thanks! I was not at all aware of those ethtool flags.
>>>> Am having a look. It definitely looks promising.
>>> Okay, I made that change. That is indeed much cleaner, thanks.
>>> Let me send the patch, initially as RFC.
>>>
>>> I've observed one issue where if we toggle the flag before bringing
>>> up the device, it hits a kernel BUG at include/linux/netdevice.h:515
>>>
>>> BUG_ON(!test_bit(NAPI_STATE_SCHED, &n->state));
>> This reminds me that we need to check netif_running() before trying to
>> enable and disable tx napi in ethtool_set_coalesce().
> The first iteration of my patch checked IFF_UP and effectively
> only allowed the change when not running. What do you mean
> by need to check?
I mean if device is not up, there's no need to toggle napi state and tx
lock.
>
> And to respond to the other follow-up notes at once:
>
>> Consider we may have interrupt moderation in the future, I tend to use
>> set_coalesce. Otherwise we may need two steps to enable moderation:
>>
>> - tx-napi on
>> - set_coalesce
> FWIW, I don't care strongly whether we do this through coalesce or priv_flags.
Ok.
>>> + if (!napi_weight)
>>> + virtqueue_enable_cb(vi->sq[i].vq);
>> I don't get why we need to disable enable cb here.
> To avoid entering no-napi mode with too few descriptors to
> make progress and no way to get out of that state. This is a
> pretty crude attempt at handling that, admittedly.
But in this case, we will call enable_cb_delayed() and we will finally
get a interrupt?
Thanks
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Willem de Bruijn @ 2018-09-14 3:40 UTC (permalink / raw)
To: Jason Wang
Cc: Florian Fainelli, Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <6db3c755-a1dc-dcc9-e110-bfc38143e83d@redhat.com>
On Thu, Sep 13, 2018 at 11:27 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年09月13日 22:58, Willem de Bruijn wrote:
> > On Thu, Sep 13, 2018 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >>
> >> On 2018年09月13日 07:27, Willem de Bruijn wrote:
> >>> On Wed, Sep 12, 2018 at 3:11 PM Willem de Bruijn
> >>> <willemdebruijn.kernel@gmail.com> wrote:
> >>>> On Wed, Sep 12, 2018 at 2:16 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>>
> >>>>> On 9/12/2018 11:07 AM, Willem de Bruijn wrote:
> >>>>>> On Wed, Sep 12, 2018 at 1:42 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>>>>
> >>>>>>> On 9/9/2018 3:44 PM, Willem de Bruijn wrote:
> >>>>>>>> From: Willem de Bruijn <willemb@google.com>
> >>>>>>>>
> >>>>>>>> Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
> >>>>>>>> Interrupt moderation is currently not supported, so these accept and
> >>>>>>>> display the default settings of 0 usec and 1 frame.
> >>>>>>>>
> >>>>>>>> Toggle tx napi through a bit in tx-frames. So as to not interfere
> >>>>>>>> with possible future interrupt moderation, use bit 10, well outside
> >>>>>>>> the reasonable range of real interrupt moderation values.
> >>>>>>>>
> >>>>>>>> Changes are not atomic. The tx IRQ, napi BH and transmit path must
> >>>>>>>> be quiesced when switching modes. Only allow changing this setting
> >>>>>>>> when the device is down.
> >>>>>>> Humm, would not a private ethtool flag to switch TX NAPI on/off be more
> >>>>>>> appropriate rather than use the coalescing configuration API here?
> >>>>>> What do you mean by private ethtool flag? A new field in ethtool
> >>>>>> --features (-k)?
> >>>>> I meant using ethtool_drvinfo::n_priv_flags, ETH_SS_PRIV_FLAGS and then
> >>>>> ETHTOOL_GFPFLAGS and ETHTOOL_SPFLAGS to control the toggling of that
> >>>>> private flag. mlx5 has a number of privates flags for instance.
> >>>> Interesting, thanks! I was not at all aware of those ethtool flags.
> >>>> Am having a look. It definitely looks promising.
> >>> Okay, I made that change. That is indeed much cleaner, thanks.
> >>> Let me send the patch, initially as RFC.
> >>>
> >>> I've observed one issue where if we toggle the flag before bringing
> >>> up the device, it hits a kernel BUG at include/linux/netdevice.h:515
> >>>
> >>> BUG_ON(!test_bit(NAPI_STATE_SCHED, &n->state));
> >> This reminds me that we need to check netif_running() before trying to
> >> enable and disable tx napi in ethtool_set_coalesce().
> > The first iteration of my patch checked IFF_UP and effectively
> > only allowed the change when not running. What do you mean
> > by need to check?
>
> I mean if device is not up, there's no need to toggle napi state and tx
> lock.
>
> >
> > And to respond to the other follow-up notes at once:
> >
> >> Consider we may have interrupt moderation in the future, I tend to use
> >> set_coalesce. Otherwise we may need two steps to enable moderation:
> >>
> >> - tx-napi on
> >> - set_coalesce
> > FWIW, I don't care strongly whether we do this through coalesce or priv_flags.
>
> Ok.
Since you prefer coalesce, let's go with that (and a revision of your
latest patch).
>
> >>> + if (!napi_weight)
> >>> + virtqueue_enable_cb(vi->sq[i].vq);
> >> I don't get why we need to disable enable cb here.
> > To avoid entering no-napi mode with too few descriptors to
> > make progress and no way to get out of that state. This is a
> > pretty crude attempt at handling that, admittedly.
>
> But in this case, we will call enable_cb_delayed() and we will finally
> get a interrupt?
Right. It's a bit of a roundabout way to ensure that
netif_tx_wake_queue and thus eventually free_old_xmit_skbs are called.
It might make more sense to just wake the device without going through
an interrupt.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Jason Wang @ 2018-09-14 3:53 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Florian Fainelli, Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <CAF=yD-Lp+MDnxLBFhG3HYONDUA9SRDDeCRUYTgZtbvWJtdO82w@mail.gmail.com>
On 2018年09月14日 11:40, Willem de Bruijn wrote:
> On Thu, Sep 13, 2018 at 11:27 PM Jason Wang <jasowang@redhat.com> wrote:
>>
>>
>> On 2018年09月13日 22:58, Willem de Bruijn wrote:
>>> On Thu, Sep 13, 2018 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote:
>>>>
>>>> On 2018年09月13日 07:27, Willem de Bruijn wrote:
>>>>> On Wed, Sep 12, 2018 at 3:11 PM Willem de Bruijn
>>>>> <willemdebruijn.kernel@gmail.com> wrote:
>>>>>> On Wed, Sep 12, 2018 at 2:16 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>>> On 9/12/2018 11:07 AM, Willem de Bruijn wrote:
>>>>>>>> On Wed, Sep 12, 2018 at 1:42 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
>>>>>>>>> On 9/9/2018 3:44 PM, Willem de Bruijn wrote:
>>>>>>>>>> From: Willem de Bruijn <willemb@google.com>
>>>>>>>>>>
>>>>>>>>>> Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
>>>>>>>>>> Interrupt moderation is currently not supported, so these accept and
>>>>>>>>>> display the default settings of 0 usec and 1 frame.
>>>>>>>>>>
>>>>>>>>>> Toggle tx napi through a bit in tx-frames. So as to not interfere
>>>>>>>>>> with possible future interrupt moderation, use bit 10, well outside
>>>>>>>>>> the reasonable range of real interrupt moderation values.
>>>>>>>>>>
>>>>>>>>>> Changes are not atomic. The tx IRQ, napi BH and transmit path must
>>>>>>>>>> be quiesced when switching modes. Only allow changing this setting
>>>>>>>>>> when the device is down.
>>>>>>>>> Humm, would not a private ethtool flag to switch TX NAPI on/off be more
>>>>>>>>> appropriate rather than use the coalescing configuration API here?
>>>>>>>> What do you mean by private ethtool flag? A new field in ethtool
>>>>>>>> --features (-k)?
>>>>>>> I meant using ethtool_drvinfo::n_priv_flags, ETH_SS_PRIV_FLAGS and then
>>>>>>> ETHTOOL_GFPFLAGS and ETHTOOL_SPFLAGS to control the toggling of that
>>>>>>> private flag. mlx5 has a number of privates flags for instance.
>>>>>> Interesting, thanks! I was not at all aware of those ethtool flags.
>>>>>> Am having a look. It definitely looks promising.
>>>>> Okay, I made that change. That is indeed much cleaner, thanks.
>>>>> Let me send the patch, initially as RFC.
>>>>>
>>>>> I've observed one issue where if we toggle the flag before bringing
>>>>> up the device, it hits a kernel BUG at include/linux/netdevice.h:515
>>>>>
>>>>> BUG_ON(!test_bit(NAPI_STATE_SCHED, &n->state));
>>>> This reminds me that we need to check netif_running() before trying to
>>>> enable and disable tx napi in ethtool_set_coalesce().
>>> The first iteration of my patch checked IFF_UP and effectively
>>> only allowed the change when not running. What do you mean
>>> by need to check?
>> I mean if device is not up, there's no need to toggle napi state and tx
>> lock.
>>
>>> And to respond to the other follow-up notes at once:
>>>
>>>> Consider we may have interrupt moderation in the future, I tend to use
>>>> set_coalesce. Otherwise we may need two steps to enable moderation:
>>>>
>>>> - tx-napi on
>>>> - set_coalesce
>>> FWIW, I don't care strongly whether we do this through coalesce or priv_flags.
>> Ok.
> Since you prefer coalesce, let's go with that (and a revision of your
> latest patch).
Good to know this.
>>>>> + if (!napi_weight)
>>>>> + virtqueue_enable_cb(vi->sq[i].vq);
>>>> I don't get why we need to disable enable cb here.
>>> To avoid entering no-napi mode with too few descriptors to
>>> make progress and no way to get out of that state. This is a
>>> pretty crude attempt at handling that, admittedly.
>> But in this case, we will call enable_cb_delayed() and we will finally
>> get a interrupt?
> Right. It's a bit of a roundabout way to ensure that
> netif_tx_wake_queue and thus eventually free_old_xmit_skbs are called.
> It might make more sense to just wake the device without going through
> an interrupt.
I'm not sure I get this. If we don't enable tx napi, we tend to delay TX
interrupt if we found the ring is about to full to avoid interrupt
storm, so we're probably ok in this case.
Thanks
^ permalink raw reply
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-14 4:11 UTC (permalink / raw)
To: Peter Oskolkov, David Miller, netdev
In-Reply-To: <20180913203814.189698-1-posk@google.com>
On 9/13/18 1:38 PM, Peter Oskolkov wrote:
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 3eed045c65a5..a3902f805305 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>
> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> {
> - rt->dst.flags |= fib6_info_dst_flags(ort);
> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
argument is passed to ip6_dst_alloc which is always invoked before
ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
>
> if (ort->fib6_flags & RTF_REJECT) {
> ip6_rt_init_dst_reject(rt, ort);
>
^ permalink raw reply
* Re: [PATCH net-next 0/8] bnxt_en: devlink param updates
From: Vasundhara Volam @ 2018-09-14 4:17 UTC (permalink / raw)
To: jakub.kicinski
Cc: David Miller, michael.chan@broadcom.com, Netdev, alexander.duyck
In-Reply-To: <20180912115026.6c05ab5e@cakuba>
On Wed, Sep 12, 2018 at 3:20 PM Jakub Kicinski
<jakub.kicinski@netronome.com> wrote:
>
> On Wed, 12 Sep 2018 12:09:37 +0530, Vasundhara Volam wrote:
> > On Tue, Sep 11, 2018 at 5:04 PM Jakub Kicinski wrote:
> > > On Tue, 11 Sep 2018 14:14:57 +0530, Vasundhara Volam wrote:
> > > > This patchset adds support for 4 generic and 1 driver-specific devlink
> > > > parameters.
> > > >
> > > > Also, this patchset adds support to return proper error code if
> > > > HWRM_NVM_GET/SET_VARIABLE commands return error code
> > > > HWRM_ERR_CODE_RESOURCE_ACCESS_DENIED.
> > > >
> > > > Vasundhara Volam (8):
> > > > devlink: Add generic parameter hw_tc_offload
> > >
> > > Much like Jiri, I can't help but wonder why do you need this?
> >
> > There is a request from our customer for a way to toggle tc_offload
> > feature in our adapter.
>
> Vasundhara, again, we don't need to know who asked you to do this, but
> _why_. What problem are you solving? What is the customer trying to
> achieve?
For Brand new big features like TC_offload, few customers are not willing
to enable it by default in the adapter(Firmware). This was a subjective decision
to disable TC_offload by default in the adapter.
>
> > > > devlink: Add generic parameter ignore_ari
> > > > devlink: Add generic parameter msix_vec_per_pf_max
> > > > devlink: Add generic parameter msix_vec_per_pf_min
> > >
> > > IMHO more structured API would be preferable if possible. The string
> > > keys won't scale if you want to set the parameters per PF, and
> > > creating more structured API for PCIe which is a relatively slow
> > > moving HW spec seems tractable.
> >
> > Sorry, could you please suggest an example? We will try to adapt.
>
> My thinking was that the same way devlink device has ports, it should
> have PCIe functions as objects which then have attributes. Instead of
> making everything a string-identified device attribute. But I'm not
> dead set on this if others don't think its a good idea.
Actually this parameters are for the port but the value given to this param
is applicable for individual PF. That's the reason I have added "per_pf" string.
If you think this is not a good idea, I can move this params to driver-specific.
^ permalink raw reply
* [PATCH v3,net-next 2/2] ip6_gre: simplify gre header parsing in ip6gre_err
From: Haishuang Yan @ 2018-09-14 4:26 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov
Cc: Jiri Benc, netdev, linux-kernel, Haishuang Yan
In-Reply-To: <1536899208-2958-1-git-send-email-yanhaishuang@cmss.chinamobile.com>
Same as ip_gre, use gre_parse_header to parse gre header in gre error
handler code.
Signed-off-by: Haishuang Yan <yanhaishuang@cmss.chinamobile.com>
---
net/ipv6/ip6_gre.c | 26 ++++----------------------
1 file changed, 4 insertions(+), 22 deletions(-)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index e493b04..515adbd 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -427,35 +427,17 @@ static void ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
u8 type, u8 code, int offset, __be32 info)
{
struct net *net = dev_net(skb->dev);
- const struct gre_base_hdr *greh;
const struct ipv6hdr *ipv6h;
- int grehlen = sizeof(*greh);
+ struct tnl_ptk_info tpi;
struct ip6_tnl *t;
- int key_off = 0;
- __be16 flags;
- __be32 key;
- if (!pskb_may_pull(skb, offset + grehlen))
- return;
- greh = (const struct gre_base_hdr *)(skb->data + offset);
- flags = greh->flags;
- if (flags & (GRE_VERSION | GRE_ROUTING))
+ if (gre_parse_header(skb, &tpi, NULL, htons(ETH_P_IPV6),
+ offset) < 0)
return;
- if (flags & GRE_CSUM)
- grehlen += 4;
- if (flags & GRE_KEY) {
- key_off = grehlen + offset;
- grehlen += 4;
- }
- if (!pskb_may_pull(skb, offset + grehlen))
- return;
ipv6h = (const struct ipv6hdr *)skb->data;
- greh = (const struct gre_base_hdr *)(skb->data + offset);
- key = key_off ? *(__be32 *)(skb->data + key_off) : 0;
-
t = ip6gre_tunnel_lookup(skb->dev, &ipv6h->daddr, &ipv6h->saddr,
- key, greh->protocol);
+ tpi.key, tpi.proto);
if (!t)
return;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net-next 0/7] add support for VSC8584 and VSC8574 Microsemi quad-port PHYs
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
Both PHYs are 4-port PHY that are 10/100/1000BASE-T, 100BASE-FX, 1000BASE-X
and triple-speed copper SFP capable, can communicate with the MAC via
SGMII, QSGMII or 1000BASE-X, supports downshifting and can set the blinking
pattern of each of its 4 LEDs, supports SyncE as well as HP Auto-MDIX
detection.
VSC8574 supports WOL and VSC8584 supports hardware offloading of MACsec.
This patch series add support for 10/100/1000BASE-T, SGMII/QSGMII link with
the MAC, downshifting, HP Auto-MDIX detection and blinking pattern for
their 4 LEDs.
They have also an internal Intel 8051 microcontroller whose firmware needs
to be patched when the PHY is reset. If the 8051's firmware has the
expected CRC, its patching can be skipped. The microcontroller can be
accessed from any port of the PHY, though the CRC function can only be done
through the PHY that is the base PHY of the package (internal address 0)
due to a limitation of the firmware.
The GPIO register bank is a set of registers that are common to all PHYs in
the package. So any modification in any register of this bank affects all
PHYs of the package.
If the PHYs haven't been reset before booting the Linux kernel and were
configured to use interrupts for e.g. link status updates, it is required
to clear the interrupts mask register of all PHYs before being able to use
interrupts with any PHY. The first PHY of the package that will be init
will take care of clearing all PHYs interrupts mask registers. Thus, we
need to keep track of the init sequence in the package, if it's already
been done or if it's to be done.
Most of the init sequence of a PHY of the package is common to all PHYs in
the package, thus we use the SMI broadcast feature which enables us to
propagate a write in one register of one PHY to all PHYs in the package.
We also introduce a new development board called PCB120 which exists in
variants for VSC8584 and VSC8574 (and that's the only difference to the
best of my knowledge).
I suggest patches 1 to 4 go through net tree and patches 5 to 7 go through
MIPS tree. Patches going through net tree and those going through MIPS tree
do not depend on one another.
This patch series depends on two patch series though:
"mscc: ocelot: add support for SerDes muxing configuration"
(https://lore.kernel.org/lkml/cover.ff40d591b548a6da31716e6e600f11a303e0e643.1536912834.git-series.quentin.schulz@bootlin.com/)
"Various improvements to Microsemi PHY driver"
(https://lore.kernel.org/lkml/cover.616d15610d44a0e3d463acd8119859f243163ad2.1536913944.git-series.quentin.schulz@bootlin.com/)
specifically patch 2/5 which defines constants that are used in this patch
series.
Thanks,
Quentin
Quentin Schulz (7):
dt-bindings: net: vsc8531: add two additional LED modes for VSC8584
net: phy: mscc: add support for VSC8584 PHY
net: phy: mscc: split config_init in two functions for VSC8584
net: phy: mscc: add support for VSC8574 PHY
MIPS: mscc: ocelot: add GPIO4 pinmuxing DT node
MIPS: mscc: add DT for Ocelot PCB120
MIPS: mscc: add PCB120 to the ocelot fitImage
arch/mips/boot/dts/mscc/Makefile | 2 +-
arch/mips/boot/dts/mscc/ocelot.dtsi | 5 +-
arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 100 ++-
arch/mips/generic/Kconfig | 6 +-
arch/mips/generic/Platform | 2 +-
arch/mips/generic/board-ocelot.its.S | 40 +-
arch/mips/generic/board-ocelot_pcb123.its.S | 23 +-
drivers/net/phy/mscc.c | 1019 ++++++++++++++++++++-
include/dt-bindings/net/mscc-phy-vsc8531.h | 2 +-
9 files changed, 1171 insertions(+), 28 deletions(-)
create mode 100644 arch/mips/boot/dts/mscc/ocelot_pcb120.dts
create mode 100644 arch/mips/generic/board-ocelot.its.S
delete mode 100644 arch/mips/generic/board-ocelot_pcb123.its.S
base-commit: d9cca8eef36bb8918c9ed28574b79b7674fd36f6
--
git-series 0.9.1
^ permalink raw reply
* [PATCH net-next 1/7] dt-bindings: net: vsc8531: add two additional LED modes for VSC8584
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
The VSC8584 (and most likely other PHYs in the same generation) has two
additional LED modes that can be picked, so let's add them.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
include/dt-bindings/net/mscc-phy-vsc8531.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/dt-bindings/net/mscc-phy-vsc8531.h b/include/dt-bindings/net/mscc-phy-vsc8531.h
index 697161f..9eb2ec2 100644
--- a/include/dt-bindings/net/mscc-phy-vsc8531.h
+++ b/include/dt-bindings/net/mscc-phy-vsc8531.h
@@ -18,9 +18,11 @@
#define VSC8531_LINK_100_1000_ACTIVITY 4
#define VSC8531_LINK_10_1000_ACTIVITY 5
#define VSC8531_LINK_10_100_ACTIVITY 6
+#define VSC8584_LINK_100FX_1000X_ACTIVITY 7
#define VSC8531_DUPLEX_COLLISION 8
#define VSC8531_COLLISION 9
#define VSC8531_ACTIVITY 10
+#define VSC8584_100FX_1000X_ACTIVITY 11
#define VSC8531_AUTONEG_FAULT 12
#define VSC8531_SERIAL_MODE 13
#define VSC8531_FORCE_LED_OFF 14
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH net-next 3/7] net: phy: mscc: split config_init in two functions for VSC8584
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
Part of the config init is common between the VSC8584 and the VSC8574,
so to prepare the upcoming support for VSC8574, separate config_init
PHY-specific code to config_pre_init function which is set in the probe
function of the PHY and used in config_init.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
drivers/net/phy/mscc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index b450489..69cc3cf 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -355,6 +355,7 @@ struct vsc8531_private {
u64 *stats;
int nstats;
bool pkg_init;
+ int (*config_pre_init)(struct mii_bus *bus, int phy);
};
#ifdef CONFIG_OF_MDIO
@@ -1298,7 +1299,7 @@ static int vsc8584_config_init(struct phy_device *phydev)
*/
if (!vsc8584_is_pkg_init(phydev, base_addr,
val & PHY_ADDR_REVERSED ? 1 : 0)) {
- ret = vsc8584_config_pre_init(phydev->mdio.bus, base_addr);
+ ret = vsc8531->config_pre_init(phydev->mdio.bus, base_addr);
if (ret)
goto err;
}
@@ -1486,6 +1487,7 @@ static int vsc8584_probe(struct phy_device *phydev)
phydev->priv = vsc8531;
+ vsc8531->config_pre_init = vsc8584_config_pre_init;
vsc8531->nleds = 4;
vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
vsc8531->hw_stats = vsc8584_hw_stats;
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH net-next 4/7] net: phy: mscc: add support for VSC8574 PHY
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
The VSC8574 PHY is a 4-ports PHY that is 10/100/1000BASE-T, 100BASE-FX,
1000BASE-X and triple-speed copper SFP capable, can communicate with
the MAC via SGMII, QSGMII or 1000BASE-X, supports WOL, downshifting and
can set the blinking pattern of each of its 4 LEDs, supports SyncE as
well as HP Auto-MDIX detection.
This adds support for 10/100/1000BASE-T, SGMII/QSGMII link with the MAC,
WOL, downshifting, HP Auto-MDIX detection and blinking pattern for its 4
LEDs.
The VSC8574 has also an internal Intel 8051 microcontroller whose
firmware needs to be patched when the PHY is reset. If the 8051's
firmware has the expected CRC, its patching can be skipped. The
microcontroller can be accessed from any port of the PHY, though the CRC
function can only be done through the PHY that is the base PHY of the
package (internal address 0) due to a limitation of the firmware.
The GPIO register bank is a set of registers that are common to all PHYs
in the package. So any modification in any register of this bank affects
all PHYs of the package.
If the PHYs haven't been reset before booting the Linux kernel and were
configured to use interrupts for e.g. link status updates, it is
required to clear the interrupts mask register of all PHYs before being
able to use interrupts with any PHY. The first PHY of the package that
will be init will take care of clearing all PHYs interrupts mask
registers. Thus, we need to keep track of the init sequence in the
package, if it's already been done or if it's to be done.
Most of the init sequence of a PHY of the package is common to all PHYs
in the package, thus we use the SMI broadcast feature which enables us
to propagate a write in one register of one PHY to all PHYs in the
package.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
drivers/net/phy/mscc.c | 303 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 303 insertions(+)
diff --git a/drivers/net/phy/mscc.c b/drivers/net/phy/mscc.c
index 69cc3cf..2289d0a 100644
--- a/drivers/net/phy/mscc.c
+++ b/drivers/net/phy/mscc.c
@@ -65,6 +65,8 @@ enum rgmii_rx_clock_delay {
#define MEDIA_OP_MODE_AMS_COPPER_100BASEFX 7
#define MEDIA_OP_MODE_POS 8
+#define MSCC_PHY_EXT_PHY_CNTL_2 24
+
#define MII_VSC85XX_INT_MASK 25
#define MII_VSC85XX_INT_MASK_MASK 0xa000
#define MII_VSC85XX_INT_MASK_WOL 0x0040
@@ -151,6 +153,7 @@ enum rgmii_rx_clock_delay {
#define DW8051_CLK_EN 0x0010
#define MICRO_CLK_EN 0x0008
#define MICRO_CLK_DIVIDE(x) ((x) >> 1)
+#define MSCC_DW8051_VLD_MASK 0xf1ff
/* x Address in range 1-4 */
#define MSCC_TRAP_ROM_ADDR(x) ((x) * 2 + 1)
@@ -184,7 +187,9 @@ enum rgmii_rx_clock_delay {
#define PROC_CMD_SGMII_MAC 0x0030
#define PROC_CMD_QSGMII_MAC 0x0020
#define PROC_CMD_NO_MAC_CONF 0x0000
+#define PROC_CMD_1588_DEFAULT_INIT 0x0010
#define PROC_CMD_NOP 0x000f
+#define PROC_CMD_PHY_INIT 0x000a
#define PROC_CMD_CRC16 0x0008
#define PROC_CMD_FIBER_MEDIA_CONF 0x0001
#define PROC_CMD_MCB_ACCESS_MAC_CONF 0x0000
@@ -198,6 +203,9 @@ enum rgmii_rx_clock_delay {
/* Test page Registers */
#define MSCC_PHY_TEST_PAGE_5 5
#define MSCC_PHY_TEST_PAGE_8 8
+#define MSCC_PHY_TEST_PAGE_9 9
+#define MSCC_PHY_TEST_PAGE_20 20
+#define MSCC_PHY_TEST_PAGE_24 24
/* Token ring page Registers */
#define MSCC_PHY_TR_CNTL 16
@@ -211,6 +219,7 @@ enum rgmii_rx_clock_delay {
#define PHY_ID_VSC8531 0x00070570
#define PHY_ID_VSC8540 0x00070760
#define PHY_ID_VSC8541 0x00070770
+#define PHY_ID_VSC8574 0x000704a0
#define PHY_ID_VSC8584 0x000707c0
#define MSCC_VDDMAC_1500 1500
@@ -258,6 +267,10 @@ enum rgmii_rx_clock_delay {
#define MSCC_VSC8584_REVB_INT8051_FW_START_ADDR 0xe800
#define MSCC_VSC8584_REVB_INT8051_FW_CRC 0xfb48
+#define MSCC_VSC8574_REVB_INT8051_FW "mscc_vsc8574_revb_int8051_29e8.bin"
+#define MSCC_VSC8574_REVB_INT8051_FW_START_ADDR 0x4000
+#define MSCC_VSC8574_REVB_INT8051_FW_CRC 0x29e8
+
#define VSC8584_REVB 0x0001
#define MSCC_DEV_REV_MASK GENMASK(3, 0)
@@ -1084,6 +1097,243 @@ static int vsc8584_patch_fw(struct mii_bus *bus, int phy,
}
/* bus->mdio_lock should be locked when using this function */
+static bool vsc8574_is_serdes_init(struct mii_bus *bus, int phy)
+{
+ u16 reg;
+ bool ret;
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
+ MSCC_PHY_PAGE_EXTENDED_GPIO);
+
+ reg = __mdiobus_read(bus, phy, MSCC_TRAP_ROM_ADDR(1));
+ if (reg != 0x3eb7) {
+ ret = false;
+ goto out;
+ }
+
+ reg = __mdiobus_read(bus, phy, MSCC_PATCH_RAM_ADDR(1));
+ if (reg != 0x4012) {
+ ret = false;
+ goto out;
+ }
+
+ reg = __mdiobus_read(bus, phy, MSCC_INT_MEM_CNTL);
+ if (reg != EN_PATCH_RAM_TRAP_ADDR(1)) {
+ ret = false;
+ goto out;
+ }
+
+ reg = __mdiobus_read(bus, phy, MSCC_DW8051_CNTL_STATUS);
+ if ((MICRO_NSOFT_RESET | RUN_FROM_INT_ROM | DW8051_CLK_EN |
+ MICRO_CLK_EN) != (reg & MSCC_DW8051_VLD_MASK)) {
+ ret = false;
+ goto out;
+ }
+
+ ret = true;
+out:
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
+
+ return ret;
+}
+
+/* bus->mdio_lock should be locked when using this function */
+static int vsc8574_config_pre_init(struct mii_bus *bus, int phy)
+{
+ struct device *dev = &bus->mdio_map[phy]->dev;
+ const struct firmware *fw;
+ u16 crc, reg;
+ bool serdes_init;
+ int ret;
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
+
+ /* all writes below this line are broadcasted to all PHYs */
+ reg = __mdiobus_read(bus, phy, MSCC_PHY_EXT_CNTL_STATUS);
+ reg |= SMI_BROADCAST_WR_EN;
+ __mdiobus_write(bus, phy, MSCC_PHY_EXT_CNTL_STATUS, reg);
+
+ __mdiobus_write(bus, phy, MII_VSC85XX_INT_MASK, 0);
+
+ /* The below register writes are tweaking analog and electrical
+ * configuration that were determined through characterization by PHY
+ * engineers. These don't mean anything more than "these are the best
+ * values".
+ */
+ __mdiobus_write(bus, phy, MSCC_PHY_EXT_PHY_CNTL_2, 0x0040);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST);
+
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_20, 0x4320);
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_24, 0x0c00);
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_9, 0x18ca);
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_5, 0x1b20);
+
+ reg = __mdiobus_read(bus, phy, MSCC_PHY_TEST_PAGE_8);
+ reg |= 0x8000;
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_8, reg);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR);
+
+ vsc8584_csr_write(bus, phy, 0x8fae, 0x000401bd);
+ vsc8584_csr_write(bus, phy, 0x8fac, 0x000f000f);
+ vsc8584_csr_write(bus, phy, 0x97a0, 0x00a0f147);
+ vsc8584_csr_write(bus, phy, 0x8fe4, 0x00052f54);
+ vsc8584_csr_write(bus, phy, 0x9792, 0x0027303d);
+ vsc8584_csr_write(bus, phy, 0x87fe, 0x00000704);
+ vsc8584_csr_write(bus, phy, 0x8fe0, 0x00060150);
+ vsc8584_csr_write(bus, phy, 0x8f82, 0x0012b00a);
+ vsc8584_csr_write(bus, phy, 0x8f80, 0x00000d74);
+ vsc8584_csr_write(bus, phy, 0x82e0, 0x00000012);
+ vsc8584_csr_write(bus, phy, 0x83a2, 0x00050208);
+ vsc8584_csr_write(bus, phy, 0x83b2, 0x00009186);
+ vsc8584_csr_write(bus, phy, 0x8fb0, 0x000e3700);
+ vsc8584_csr_write(bus, phy, 0x9688, 0x00049f81);
+ vsc8584_csr_write(bus, phy, 0x8fd2, 0x0000ffff);
+ vsc8584_csr_write(bus, phy, 0x968a, 0x00039fa2);
+ vsc8584_csr_write(bus, phy, 0x9690, 0x0020640b);
+ vsc8584_csr_write(bus, phy, 0x8258, 0x00002220);
+ vsc8584_csr_write(bus, phy, 0x825a, 0x00002a20);
+ vsc8584_csr_write(bus, phy, 0x825c, 0x00003060);
+ vsc8584_csr_write(bus, phy, 0x825e, 0x00003fa0);
+ vsc8584_csr_write(bus, phy, 0x83a6, 0x0000e0f0);
+ vsc8584_csr_write(bus, phy, 0x8f92, 0x00001489);
+ vsc8584_csr_write(bus, phy, 0x96a2, 0x00007000);
+ vsc8584_csr_write(bus, phy, 0x96a6, 0x00071448);
+ vsc8584_csr_write(bus, phy, 0x96a0, 0x00eeffdd);
+ vsc8584_csr_write(bus, phy, 0x8fe8, 0x0091b06c);
+ vsc8584_csr_write(bus, phy, 0x8fea, 0x00041600);
+ vsc8584_csr_write(bus, phy, 0x96b0, 0x00eeff00);
+ vsc8584_csr_write(bus, phy, 0x96b2, 0x00007000);
+ vsc8584_csr_write(bus, phy, 0x96b4, 0x00000814);
+ vsc8584_csr_write(bus, phy, 0x8f90, 0x00688980);
+ vsc8584_csr_write(bus, phy, 0x83a4, 0x0000d8f0);
+ vsc8584_csr_write(bus, phy, 0x8fc0, 0x00000400);
+ vsc8584_csr_write(bus, phy, 0x87fa, 0x0050100f);
+ vsc8584_csr_write(bus, phy, 0x8796, 0x00000003);
+ vsc8584_csr_write(bus, phy, 0x87f8, 0x00c3ff98);
+ vsc8584_csr_write(bus, phy, 0x8fa4, 0x0018292a);
+ vsc8584_csr_write(bus, phy, 0x968c, 0x00d2c46f);
+ vsc8584_csr_write(bus, phy, 0x97a2, 0x00000620);
+ vsc8584_csr_write(bus, phy, 0x96a4, 0x0013132f);
+ vsc8584_csr_write(bus, phy, 0x96a8, 0x00000000);
+ vsc8584_csr_write(bus, phy, 0x8ffc, 0x00c0a028);
+ vsc8584_csr_write(bus, phy, 0x8fec, 0x00901c09);
+ vsc8584_csr_write(bus, phy, 0x8fee, 0x0004a6a1);
+ vsc8584_csr_write(bus, phy, 0x8ffe, 0x00b01807);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
+ MSCC_PHY_PAGE_EXTENDED_2);
+
+ __mdiobus_write(bus, phy, MSCC_PHY_CU_PMD_TX_CNTL, 0x028e);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TR);
+
+ vsc8584_csr_write(bus, phy, 0x8486, 0x0008a518);
+ vsc8584_csr_write(bus, phy, 0x8488, 0x006dc696);
+ vsc8584_csr_write(bus, phy, 0x848a, 0x00000912);
+ vsc8584_csr_write(bus, phy, 0x848e, 0x00000db6);
+ vsc8584_csr_write(bus, phy, 0x849c, 0x00596596);
+ vsc8584_csr_write(bus, phy, 0x849e, 0x00000514);
+ vsc8584_csr_write(bus, phy, 0x84a2, 0x00410280);
+ vsc8584_csr_write(bus, phy, 0x84a4, 0x00000000);
+ vsc8584_csr_write(bus, phy, 0x84a6, 0x00000000);
+ vsc8584_csr_write(bus, phy, 0x84a8, 0x00000000);
+ vsc8584_csr_write(bus, phy, 0x84aa, 0x00000000);
+ vsc8584_csr_write(bus, phy, 0x84ae, 0x007df7dd);
+ vsc8584_csr_write(bus, phy, 0x84b0, 0x006d95d4);
+ vsc8584_csr_write(bus, phy, 0x84b2, 0x00492410);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_TEST);
+
+ reg = __mdiobus_read(bus, phy, MSCC_PHY_TEST_PAGE_8);
+ reg &= ~0x8000;
+ __mdiobus_write(bus, phy, MSCC_PHY_TEST_PAGE_8, reg);
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
+ MSCC_PHY_PAGE_STANDARD);
+
+ /* end of write broadcasting */
+ reg = __mdiobus_read(bus, phy, MSCC_PHY_EXT_CNTL_STATUS);
+ reg &= ~SMI_BROADCAST_WR_EN;
+ __mdiobus_write(bus, phy, MSCC_PHY_EXT_CNTL_STATUS, reg);
+
+ ret = request_firmware(&fw, MSCC_VSC8574_REVB_INT8051_FW, dev);
+ if (ret) {
+ dev_err(dev, "failed to load firmware %s, ret: %d\n",
+ MSCC_VSC8574_REVB_INT8051_FW, ret);
+ return ret;
+ }
+
+ /* Add one byte to size for the one added by the patch_fw function */
+ ret = vsc8584_get_fw_crc(bus, phy,
+ MSCC_VSC8574_REVB_INT8051_FW_START_ADDR,
+ fw->size + 1, &crc);
+ if (ret)
+ goto out;
+
+ if (crc == MSCC_VSC8574_REVB_INT8051_FW_CRC) {
+ serdes_init = vsc8574_is_serdes_init(bus, phy);
+
+ if (!serdes_init) {
+ ret = vsc8584_micro_assert_reset(bus, phy);
+ if (ret) {
+ dev_err(dev,
+ "%s: failed to assert reset of micro\n",
+ __func__);
+ return ret;
+ }
+ }
+ } else {
+ dev_dbg(dev, "FW CRC is not the expected one, patching FW\n");
+
+ serdes_init = false;
+
+ if (vsc8584_patch_fw(bus, phy, fw))
+ dev_warn(dev,
+ "failed to patch FW, expect non-optimal device\n");
+ }
+
+ if (!serdes_init) {
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
+ MSCC_PHY_PAGE_EXTENDED_GPIO);
+
+ __mdiobus_write(bus, phy, MSCC_TRAP_ROM_ADDR(1), 0x3eb7);
+ __mdiobus_write(bus, phy, MSCC_PATCH_RAM_ADDR(1), 0x4012);
+ __mdiobus_write(bus, phy, MSCC_INT_MEM_CNTL,
+ EN_PATCH_RAM_TRAP_ADDR(1));
+
+ vsc8584_micro_deassert_reset(bus, phy, false);
+
+ /* Add one byte to size for the one added by the patch_fw
+ * function
+ */
+ ret = vsc8584_get_fw_crc(bus, phy,
+ MSCC_VSC8574_REVB_INT8051_FW_START_ADDR,
+ fw->size + 1, &crc);
+ if (ret)
+ goto out;
+
+ if (crc != MSCC_VSC8574_REVB_INT8051_FW_CRC)
+ dev_warn(dev,
+ "FW CRC after patching is not the expected one, expect non-optimal device\n");
+ }
+
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS,
+ MSCC_PHY_PAGE_EXTENDED_GPIO);
+
+ ret = vsc8584_cmd(bus, phy, PROC_CMD_1588_DEFAULT_INIT |
+ PROC_CMD_PHY_INIT);
+
+out:
+ __mdiobus_write(bus, phy, MSCC_EXT_PAGE_ACCESS, MSCC_PHY_PAGE_STANDARD);
+
+ release_firmware(fw);
+
+ return ret;
+}
+
+/* bus->mdio_lock should be locked when using this function */
static int vsc8584_config_pre_init(struct mii_bus *bus, int phy)
{
struct device *dev = &bus->mdio_map[phy]->dev;
@@ -1469,6 +1719,33 @@ static int vsc85xx_read_status(struct phy_device *phydev)
return genphy_read_status(phydev);
}
+static int vsc8574_probe(struct phy_device *phydev)
+{
+ struct vsc8531_private *vsc8531;
+ u32 default_mode[4] = {VSC8531_LINK_1000_ACTIVITY,
+ VSC8531_LINK_100_ACTIVITY, VSC8531_LINK_ACTIVITY,
+ VSC8531_DUPLEX_COLLISION};
+
+ vsc8531 = devm_kzalloc(&phydev->mdio.dev, sizeof(*vsc8531), GFP_KERNEL);
+ if (!vsc8531)
+ return -ENOMEM;
+
+ phydev->priv = vsc8531;
+
+ vsc8531->config_pre_init = vsc8574_config_pre_init;
+ vsc8531->nleds = 4;
+ vsc8531->supp_led_modes = VSC8584_SUPP_LED_MODES;
+ vsc8531->hw_stats = vsc8584_hw_stats;
+ vsc8531->nstats = ARRAY_SIZE(vsc8584_hw_stats);
+ vsc8531->stats = devm_kzalloc(&phydev->mdio.dev,
+ sizeof(u64) * vsc8531->nstats,
+ GFP_KERNEL);
+ if (!vsc8531->stats)
+ return -ENOMEM;
+
+ return vsc85xx_dt_led_modes_get(phydev, default_mode);
+}
+
static int vsc8584_probe(struct phy_device *phydev)
{
struct vsc8531_private *vsc8531;
@@ -1631,6 +1908,31 @@ static struct phy_driver vsc85xx_driver[] = {
.get_stats = &vsc85xx_get_stats,
},
{
+ .phy_id = PHY_ID_VSC8574,
+ .name = "Microsemi GE VSC8574 SyncE",
+ .phy_id_mask = 0xfffffff0,
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .soft_reset = &genphy_soft_reset,
+ .config_init = &vsc8584_config_init,
+ .config_aneg = &vsc85xx_config_aneg,
+ .aneg_done = &genphy_aneg_done,
+ .read_status = &vsc85xx_read_status,
+ .ack_interrupt = &vsc85xx_ack_interrupt,
+ .config_intr = &vsc85xx_config_intr,
+ .did_interrupt = &vsc8584_did_interrupt,
+ .suspend = &genphy_suspend,
+ .resume = &genphy_resume,
+ .probe = &vsc8574_probe,
+ .set_wol = &vsc85xx_wol_set,
+ .get_wol = &vsc85xx_wol_get,
+ .get_tunable = &vsc85xx_get_tunable,
+ .set_tunable = &vsc85xx_set_tunable,
+ .get_sset_count = &vsc85xx_get_sset_count,
+ .get_strings = &vsc85xx_get_strings,
+ .get_stats = &vsc85xx_get_stats,
+},
+{
.phy_id = PHY_ID_VSC8584,
.name = "Microsemi GE VSC8584 SyncE",
.phy_id_mask = 0xfffffff0,
@@ -1663,6 +1965,7 @@ static struct mdio_device_id __maybe_unused vsc85xx_tbl[] = {
{ PHY_ID_VSC8531, 0xfffffff0, },
{ PHY_ID_VSC8540, 0xfffffff0, },
{ PHY_ID_VSC8541, 0xfffffff0, },
+ { PHY_ID_VSC8574, 0xfffffff0, },
{ PHY_ID_VSC8584, 0xfffffff0, },
{ }
};
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 5/7] MIPS: mscc: ocelot: add GPIO4 pinmuxing DT node
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
In order to use GPIO4 as a GPIO, we need to mux it in this mode so let's
declare a new pinctrl DT node for it.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
arch/mips/boot/dts/mscc/ocelot.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/mips/boot/dts/mscc/ocelot.dtsi b/arch/mips/boot/dts/mscc/ocelot.dtsi
index 8ce317c..b5c4c74 100644
--- a/arch/mips/boot/dts/mscc/ocelot.dtsi
+++ b/arch/mips/boot/dts/mscc/ocelot.dtsi
@@ -182,6 +182,11 @@
interrupts = <13>;
#interrupt-cells = <2>;
+ gpio4: gpio4 {
+ pins = "GPIO_4";
+ function = "gpio";
+ };
+
uart_pins: uart-pins {
pins = "GPIO_6", "GPIO_7";
function = "uart";
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 6/7] MIPS: mscc: add DT for Ocelot PCB120
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
The Ocelot PCB120 evaluation board is different from the PCB123 in that
it has 4 external VSC8584 (or VSC8574) PHYs.
It uses the SoC's second MDIO bus for external PHYs which have a
reversed address on the bus (i.e. PHY4 is on address 3, PHY5 is on
address 2, PHY6 on 1 and PHY7 on 0).
Here is how the PHYs are connected to the switch ports:
port 0: phy0 (internal)
port 1: phy1 (internal)
port 2: phy2 (internal)
port 3: phy3 (internal)
port 4: phy7
port 5: phy4
port 6: phy6
port 9: phy5
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
arch/mips/boot/dts/mscc/Makefile | 2 +-
arch/mips/boot/dts/mscc/ocelot_pcb120.dts | 100 +++++++++++++++++++++++-
2 files changed, 101 insertions(+), 1 deletion(-)
create mode 100644 arch/mips/boot/dts/mscc/ocelot_pcb120.dts
diff --git a/arch/mips/boot/dts/mscc/Makefile b/arch/mips/boot/dts/mscc/Makefile
index 9a9bb7e..ec6f5b2 100644
--- a/arch/mips/boot/dts/mscc/Makefile
+++ b/arch/mips/boot/dts/mscc/Makefile
@@ -1,3 +1,3 @@
-dtb-$(CONFIG_MSCC_OCELOT) += ocelot_pcb123.dtb
+dtb-$(CONFIG_MSCC_OCELOT) += ocelot_pcb123.dtb ocelot_pcb120.dtb
obj-$(CONFIG_BUILTIN_DTB) += $(addsuffix .o, $(dtb-y))
diff --git a/arch/mips/boot/dts/mscc/ocelot_pcb120.dts b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
new file mode 100644
index 0000000..8eb03a5
--- /dev/null
+++ b/arch/mips/boot/dts/mscc/ocelot_pcb120.dts
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: (GPL-2.0 OR MIT)
+/* Copyright (c) 2017 Microsemi Corporation */
+
+/dts-v1/;
+
+#include <dt-bindings/interrupt-controller/irq.h>
+#include <dt-bindings/phy/phy-ocelot-serdes.h>
+#include "ocelot.dtsi"
+
+/ {
+ compatible = "mscc,ocelot-pcb120", "mscc,ocelot";
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ memory@0 {
+ device_type = "memory";
+ reg = <0x0 0x0e000000>;
+ };
+};
+
+&mdio0 {
+ status = "okay";
+};
+
+&mdio1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&miim1>, <&gpio4>;
+
+ phy7: ethernet-phy@0 {
+ reg = <0>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio>;
+ };
+ phy6: ethernet-phy@1 {
+ reg = <1>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio>;
+ };
+ phy5: ethernet-phy@2 {
+ reg = <2>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio>;
+ };
+ phy4: ethernet-phy@3 {
+ reg = <3>;
+ interrupts = <4 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-parent = <&gpio>;
+ };
+};
+
+&port0 {
+ phy-handle = <&phy0>;
+};
+
+&port1 {
+ phy-handle = <&phy1>;
+};
+
+&port2 {
+ phy-handle = <&phy2>;
+};
+
+&port3 {
+ phy-handle = <&phy3>;
+};
+
+&port4 {
+ phy-handle = <&phy7>;
+ phy-mode = "sgmii";
+ phys = <&serdes 4 SERDES1G_2>;
+};
+
+&port5 {
+ phy-handle = <&phy4>;
+ phy-mode = "sgmii";
+ phys = <&serdes 5 SERDES1G_5>;
+};
+
+&port6 {
+ phy-handle = <&phy6>;
+ phy-mode = "sgmii";
+ phys = <&serdes 6 SERDES1G_3>;
+};
+
+&port9 {
+ phy-handle = <&phy5>;
+ phy-mode = "sgmii";
+ phys = <&serdes 9 SERDES1G_4>;
+};
+
+&uart0 {
+ status = "okay";
+};
+
+&uart2 {
+ status = "okay";
+};
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH 7/7] MIPS: mscc: add PCB120 to the ocelot fitImage
From: Quentin Schulz @ 2018-09-14 9:44 UTC (permalink / raw)
To: alexandre.belloni, ralf, paul.burton, jhogan, robh+dt,
mark.rutland, davem, andrew, f.fainelli
Cc: allan.nielsen, linux-mips, devicetree, linux-kernel, netdev,
thomas.petazzoni, antoine.tenart, Quentin Schulz
In-Reply-To: <cover.b921b010b6d6bde1c11e69551ae38f3b2818645b.1536916714.git-series.quentin.schulz@bootlin.com>
PCB120 and PCB123 are both development boards based on Microsemi Ocelot
so let's use the same fitImage for both.
Signed-off-by: Quentin Schulz <quentin.schulz@bootlin.com>
---
arch/mips/generic/Kconfig | 6 +--
arch/mips/generic/Platform | 2 +-
arch/mips/generic/board-ocelot.its.S | 40 ++++++++++++++++++++++-
arch/mips/generic/board-ocelot_pcb123.its.S | 23 +-------------
4 files changed, 44 insertions(+), 27 deletions(-)
create mode 100644 arch/mips/generic/board-ocelot.its.S
delete mode 100644 arch/mips/generic/board-ocelot_pcb123.its.S
diff --git a/arch/mips/generic/Kconfig b/arch/mips/generic/Kconfig
index 08e33c6..fd60198 100644
--- a/arch/mips/generic/Kconfig
+++ b/arch/mips/generic/Kconfig
@@ -65,11 +65,11 @@ config FIT_IMAGE_FDT_XILFPGA
Enable this to include the FDT for the MIPSfpga platform
from Imagination Technologies in the FIT kernel image.
-config FIT_IMAGE_FDT_OCELOT_PCB123
- bool "Include FDT for Microsemi Ocelot PCB123"
+config FIT_IMAGE_FDT_OCELOT
+ bool "Include FDT for Microsemi Ocelot development platforms"
select MSCC_OCELOT
help
- Enable this to include the FDT for the Ocelot PCB123 platform
+ Enable this to include the FDT for the Ocelot development platforms
from Microsemi in the FIT kernel image.
This requires u-boot on the platform.
diff --git a/arch/mips/generic/Platform b/arch/mips/generic/Platform
index 879cb80..eaa19d1 100644
--- a/arch/mips/generic/Platform
+++ b/arch/mips/generic/Platform
@@ -16,5 +16,5 @@ all-$(CONFIG_MIPS_GENERIC) := vmlinux.gz.itb
its-y := vmlinux.its.S
its-$(CONFIG_FIT_IMAGE_FDT_BOSTON) += board-boston.its.S
its-$(CONFIG_FIT_IMAGE_FDT_NI169445) += board-ni169445.its.S
-its-$(CONFIG_FIT_IMAGE_FDT_OCELOT_PCB123) += board-ocelot_pcb123.its.S
+its-$(CONFIG_FIT_IMAGE_FDT_OCELOT) += board-ocelot.its.S
its-$(CONFIG_FIT_IMAGE_FDT_XILFPGA) += board-xilfpga.its.S
diff --git a/arch/mips/generic/board-ocelot.its.S b/arch/mips/generic/board-ocelot.its.S
new file mode 100644
index 0000000..3da2398
--- /dev/null
+++ b/arch/mips/generic/board-ocelot.its.S
@@ -0,0 +1,40 @@
+/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
+/ {
+ images {
+ fdt@ocelot_pcb123 {
+ description = "MSCC Ocelot PCB123 Device Tree";
+ data = /incbin/("boot/dts/mscc/ocelot_pcb123.dtb");
+ type = "flat_dt";
+ arch = "mips";
+ compression = "none";
+ hash@0 {
+ algo = "sha1";
+ };
+ };
+
+ fdt@ocelot_pcb120 {
+ description = "MSCC Ocelot PCB120 Device Tree";
+ data = /incbin/("boot/dts/mscc/ocelot_pcb120.dtb");
+ type = "flat_dt";
+ arch = "mips";
+ compression = "none";
+ hash@0 {
+ algo = "sha1";
+ };
+ };
+ };
+
+ configurations {
+ conf@ocelot_pcb123 {
+ description = "Ocelot Linux kernel";
+ kernel = "kernel@0";
+ fdt = "fdt@ocelot_pcb123";
+ };
+
+ conf@ocelot_pcb120 {
+ description = "Ocelot Linux kernel";
+ kernel = "kernel@0";
+ fdt = "fdt@ocelot_pcb120";
+ };
+ };
+};
diff --git a/arch/mips/generic/board-ocelot_pcb123.its.S b/arch/mips/generic/board-ocelot_pcb123.its.S
deleted file mode 100644
index 5a7d5e1..0000000
--- a/arch/mips/generic/board-ocelot_pcb123.its.S
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
-/ {
- images {
- fdt@ocelot_pcb123 {
- description = "MSCC Ocelot PCB123 Device Tree";
- data = /incbin/("boot/dts/mscc/ocelot_pcb123.dtb");
- type = "flat_dt";
- arch = "mips";
- compression = "none";
- hash@0 {
- algo = "sha1";
- };
- };
- };
-
- configurations {
- conf@ocelot_pcb123 {
- description = "Ocelot Linux kernel";
- kernel = "kernel@0";
- fdt = "fdt@ocelot_pcb123";
- };
- };
-};
--
git-series 0.9.1
^ permalink raw reply related
* [PATCH net] veth: Orphan skb before GRO
From: Toshiaki Makita @ 2018-09-14 4:33 UTC (permalink / raw)
To: David S. Miller; +Cc: Toshiaki Makita, netdev, Paolo Abeni, Eric Dumazet
GRO expects skbs not to be owned by sockets, but when XDP is enabled veth
passed skbs owned by sockets. It caused corrupted sk_wmem_alloc.
Paolo Abeni reported the following splat:
[ 362.098904] refcount_t overflow at skb_set_owner_w+0x5e/0xa0 in iperf3[1644], uid/euid: 0/0
[ 362.108239] WARNING: CPU: 0 PID: 1644 at kernel/panic.c:648 refcount_error_report+0xa0/0xa4
[ 362.117547] Modules linked in: tcp_diag inet_diag veth intel_rapl sb_edac x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel kvm irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel intel_cstate intel_uncore intel_rapl_perf ipmi_ssif iTCO_wdt sg ipmi_si iTCO_vendor_support ipmi_devintf mxm_wmi ipmi_msghandler pcspkr dcdbas mei_me wmi mei lpc_ich acpi_power_meter pcc_cpufreq xfs libcrc32c sd_mod mgag200 drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ixgbe igb ttm ahci mdio libahci ptp crc32c_intel drm pps_core libata i2c_algo_bit dca dm_mirror dm_region_hash dm_log dm_mod
[ 362.176622] CPU: 0 PID: 1644 Comm: iperf3 Not tainted 4.19.0-rc2.vanilla+ #2025
[ 362.184777] Hardware name: Dell Inc. PowerEdge R730/072T6D, BIOS 2.1.7 06/16/2016
[ 362.193124] RIP: 0010:refcount_error_report+0xa0/0xa4
[ 362.198758] Code: 08 00 00 48 8b 95 80 00 00 00 49 8d 8c 24 80 0a 00 00 41 89 c1 44 89 2c 24 48 89 de 48 c7 c7 18 4d e7 9d 31 c0 e8 30 fa ff ff <0f> 0b eb 88 0f 1f 44 00 00 55 48 89 e5 41 56 41 55 41 54 49 89 fc
[ 362.219711] RSP: 0018:ffff9ee6ff603c20 EFLAGS: 00010282
[ 362.225538] RAX: 0000000000000000 RBX: ffffffff9de83e10 RCX: 0000000000000000
[ 362.233497] RDX: 0000000000000001 RSI: ffff9ee6ff6167d8 RDI: ffff9ee6ff6167d8
[ 362.241457] RBP: ffff9ee6ff603d78 R08: 0000000000000490 R09: 0000000000000004
[ 362.249416] R10: 0000000000000000 R11: ffff9ee6ff603990 R12: ffff9ee664b94500
[ 362.257377] R13: 0000000000000000 R14: 0000000000000004 R15: ffffffff9de615f9
[ 362.265337] FS: 00007f1d22d28740(0000) GS:ffff9ee6ff600000(0000) knlGS:0000000000000000
[ 362.274363] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 362.280773] CR2: 00007f1d222f35d0 CR3: 0000001fddfec003 CR4: 00000000001606f0
[ 362.288733] Call Trace:
[ 362.291459] <IRQ>
[ 362.293702] ex_handler_refcount+0x4e/0x80
[ 362.298269] fixup_exception+0x35/0x40
[ 362.302451] do_trap+0x109/0x150
[ 362.306048] do_error_trap+0xd5/0x130
[ 362.315766] invalid_op+0x14/0x20
[ 362.319460] RIP: 0010:skb_set_owner_w+0x5e/0xa0
[ 362.324512] Code: ef ff ff 74 49 48 c7 43 60 20 7b 4a 9d 8b 85 f4 01 00 00 85 c0 75 16 8b 83 e0 00 00 00 f0 01 85 44 01 00 00 0f 88 d8 23 16 00 <5b> 5d c3 80 8b 91 00 00 00 01 8b 85 f4 01 00 00 89 83 a4 00 00 00
[ 362.345465] RSP: 0018:ffff9ee6ff603e20 EFLAGS: 00010a86
[ 362.351291] RAX: 0000000000001100 RBX: ffff9ee65deec700 RCX: ffff9ee65e829244
[ 362.359250] RDX: 0000000000000100 RSI: ffff9ee65e829100 RDI: ffff9ee65deec700
[ 362.367210] RBP: ffff9ee65e829100 R08: 000000000002a380 R09: 0000000000000000
[ 362.375169] R10: 0000000000000002 R11: fffff1a4bf77bb00 R12: ffffc0754661d000
[ 362.383130] R13: ffff9ee65deec200 R14: ffff9ee65f597000 R15: 00000000000000aa
[ 362.391092] veth_xdp_rcv+0x4e4/0x890 [veth]
[ 362.399357] veth_poll+0x4d/0x17a [veth]
[ 362.403731] net_rx_action+0x2af/0x3f0
[ 362.407912] __do_softirq+0xdd/0x29e
[ 362.411897] do_softirq_own_stack+0x2a/0x40
[ 362.416561] </IRQ>
[ 362.418899] do_softirq+0x4b/0x70
[ 362.422594] __local_bh_enable_ip+0x50/0x60
[ 362.427258] ip_finish_output2+0x16a/0x390
[ 362.431824] ip_output+0x71/0xe0
[ 362.440670] __tcp_transmit_skb+0x583/0xab0
[ 362.445333] tcp_write_xmit+0x247/0xfb0
[ 362.449609] __tcp_push_pending_frames+0x2d/0xd0
[ 362.454760] tcp_sendmsg_locked+0x857/0xd30
[ 362.459424] tcp_sendmsg+0x27/0x40
[ 362.463216] sock_sendmsg+0x36/0x50
[ 362.467104] sock_write_iter+0x87/0x100
[ 362.471382] __vfs_write+0x112/0x1a0
[ 362.475369] vfs_write+0xad/0x1a0
[ 362.479062] ksys_write+0x52/0xc0
[ 362.482759] do_syscall_64+0x5b/0x180
[ 362.486841] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 362.492473] RIP: 0033:0x7f1d22293238
[ 362.496458] Code: 89 02 48 c7 c0 ff ff ff ff eb b3 0f 1f 80 00 00 00 00 f3 0f 1e fa 48 8d 05 c5 54 2d 00 8b 00 85 c0 75 17 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 58 c3 0f 1f 80 00 00 00 00 41 54 49 89 d4 55
[ 362.517409] RSP: 002b:00007ffebaef8008 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 362.525855] RAX: ffffffffffffffda RBX: 0000000000002800 RCX: 00007f1d22293238
[ 362.533816] RDX: 0000000000002800 RSI: 00007f1d22d36000 RDI: 0000000000000005
[ 362.541775] RBP: 00007f1d22d36000 R08: 00000002db777a30 R09: 0000562b70712b20
[ 362.549734] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000005
[ 362.557693] R13: 0000000000002800 R14: 00007ffebaef8060 R15: 0000562b70712260
In order to avoid this, orphan the skb before entering GRO.
Fixes: 948d4f214fde ("veth: Add driver XDP")
Reported-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
---
drivers/net/veth.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index 8d679c8..41a00cd 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -463,6 +463,8 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, struct sk_buff *skb,
int mac_len, delta, off;
struct xdp_buff xdp;
+ skb_orphan(skb);
+
rcu_read_lock();
xdp_prog = rcu_dereference(rq->xdp_prog);
if (unlikely(!xdp_prog)) {
@@ -508,8 +510,6 @@ static struct sk_buff *veth_xdp_rcv_skb(struct veth_rq *rq, struct sk_buff *skb,
skb_copy_header(nskb, skb);
head_off = skb_headroom(nskb) - skb_headroom(skb);
skb_headers_offset_update(nskb, head_off);
- if (skb->sk)
- skb_set_owner_w(nskb, skb->sk);
consume_skb(skb);
skb = nskb;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [RFC PATCH net-next v1 00/14] rename and shrink i40evf
From: Benjamin Poirier @ 2018-09-14 4:39 UTC (permalink / raw)
To: Jesse Brandeburg; +Cc: netdev, intel-wired-lan, jeffrey.t.kirsher
In-Reply-To: <20180913223144.75823-1-jesse.brandeburg@intel.com>
On 2018/09/13 15:31, Jesse Brandeburg wrote:
[...]
>
> ---
> v1: initial RFC
>
> Jesse Brandeburg (14):
> intel-ethernet: rename i40evf to iavf
Seems like patch 1 didn't make it to netdev
https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20180910/014025.html
> iavf: diet and reformat
> iavf: rename functions and structs to new name
> iavf: rename i40e_status to iavf_status
> iavf: move i40evf files to new name
> iavf: remove references to old names
> iavf: rename device ID defines
> iavf: rename I40E_ADMINQ_DESC
> iavf: rename i40e_hw to iavf_hw
> iavf: replace i40e_debug with iavf version
> iavf: tracing infrastructure rename
> iavf: rename most of i40e strings
> iavf: finish renaming files to iavf
> intel-ethernet: use correct module license
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-14 9:53 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Andrew Lutomirski, LKML, Netdev, David Miller, Greg Kroah-Hartman,
Samuel Neves, Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAKv+Gu8zULznCmgz++LNNTwfTifaz5L92h=vvWb4yh5fHh-huQ@mail.gmail.com>
On Fri, Sep 14, 2018 at 8:15 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> OK, so given random.c's future dependency on Zinc (for ChaCha20), and
> the fact that Zinc is one monolithic piece of code, all versions of
> all algorithms will always be statically linked into the kernel
> proper. I'm not sure that is acceptable.
v4 already addresses that issue, actually. I'll post it shortly.
> BTW you haven't answered my question yet about what happens when the
> WireGuard protocol version changes: will we need a flag day and switch
> all deployments over at the same time?
No, that won't be necessary, necessarily. Peers are individually
versioned and the protocol is fairly flexible in this regard.
^ permalink raw reply
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Willem de Bruijn @ 2018-09-14 4:46 UTC (permalink / raw)
To: Jason Wang
Cc: Florian Fainelli, Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <0af0043d-c13c-e68e-795d-ef62901f57fc@redhat.com>
On Thu, Sep 13, 2018 at 11:53 PM Jason Wang <jasowang@redhat.com> wrote:
>
>
>
> On 2018年09月14日 11:40, Willem de Bruijn wrote:
> > On Thu, Sep 13, 2018 at 11:27 PM Jason Wang <jasowang@redhat.com> wrote:
> >>
> >>
> >> On 2018年09月13日 22:58, Willem de Bruijn wrote:
> >>> On Thu, Sep 13, 2018 at 5:02 AM Jason Wang <jasowang@redhat.com> wrote:
> >>>>
> >>>> On 2018年09月13日 07:27, Willem de Bruijn wrote:
> >>>>> On Wed, Sep 12, 2018 at 3:11 PM Willem de Bruijn
> >>>>> <willemdebruijn.kernel@gmail.com> wrote:
> >>>>>> On Wed, Sep 12, 2018 at 2:16 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>>>> On 9/12/2018 11:07 AM, Willem de Bruijn wrote:
> >>>>>>>> On Wed, Sep 12, 2018 at 1:42 PM Florian Fainelli <f.fainelli@gmail.com> wrote:
> >>>>>>>>> On 9/9/2018 3:44 PM, Willem de Bruijn wrote:
> >>>>>>>>>> From: Willem de Bruijn <willemb@google.com>
> >>>>>>>>>>
> >>>>>>>>>> Implement ethtool .set_coalesce (-C) and .get_coalesce (-c) handlers.
> >>>>>>>>>> Interrupt moderation is currently not supported, so these accept and
> >>>>>>>>>> display the default settings of 0 usec and 1 frame.
> >>>>>>>>>>
> >>>>>>>>>> Toggle tx napi through a bit in tx-frames. So as to not interfere
> >>>>>>>>>> with possible future interrupt moderation, use bit 10, well outside
> >>>>>>>>>> the reasonable range of real interrupt moderation values.
> >>>>>>>>>>
> >>>>>>>>>> Changes are not atomic. The tx IRQ, napi BH and transmit path must
> >>>>>>>>>> be quiesced when switching modes. Only allow changing this setting
> >>>>>>>>>> when the device is down.
> >>>>>>>>> Humm, would not a private ethtool flag to switch TX NAPI on/off be more
> >>>>>>>>> appropriate rather than use the coalescing configuration API here?
> >>>>>>>> What do you mean by private ethtool flag? A new field in ethtool
> >>>>>>>> --features (-k)?
> >>>>>>> I meant using ethtool_drvinfo::n_priv_flags, ETH_SS_PRIV_FLAGS and then
> >>>>>>> ETHTOOL_GFPFLAGS and ETHTOOL_SPFLAGS to control the toggling of that
> >>>>>>> private flag. mlx5 has a number of privates flags for instance.
> >>>>>> Interesting, thanks! I was not at all aware of those ethtool flags.
> >>>>>> Am having a look. It definitely looks promising.
> >>>>> Okay, I made that change. That is indeed much cleaner, thanks.
> >>>>> Let me send the patch, initially as RFC.
> >>>>>
> >>>>> I've observed one issue where if we toggle the flag before bringing
> >>>>> up the device, it hits a kernel BUG at include/linux/netdevice.h:515
> >>>>>
> >>>>> BUG_ON(!test_bit(NAPI_STATE_SCHED, &n->state));
> >>>> This reminds me that we need to check netif_running() before trying to
> >>>> enable and disable tx napi in ethtool_set_coalesce().
> >>> The first iteration of my patch checked IFF_UP and effectively
> >>> only allowed the change when not running. What do you mean
> >>> by need to check?
> >> I mean if device is not up, there's no need to toggle napi state and tx
> >> lock.
> >>
> >>> And to respond to the other follow-up notes at once:
> >>>
> >>>> Consider we may have interrupt moderation in the future, I tend to use
> >>>> set_coalesce. Otherwise we may need two steps to enable moderation:
> >>>>
> >>>> - tx-napi on
> >>>> - set_coalesce
> >>> FWIW, I don't care strongly whether we do this through coalesce or priv_flags.
> >> Ok.
> > Since you prefer coalesce, let's go with that (and a revision of your
> > latest patch).
>
> Good to know this.
>
> >>>>> + if (!napi_weight)
> >>>>> + virtqueue_enable_cb(vi->sq[i].vq);
> >>>> I don't get why we need to disable enable cb here.
> >>> To avoid entering no-napi mode with too few descriptors to
> >>> make progress and no way to get out of that state. This is a
> >>> pretty crude attempt at handling that, admittedly.
> >> But in this case, we will call enable_cb_delayed() and we will finally
> >> get a interrupt?
> > Right. It's a bit of a roundabout way to ensure that
> > netif_tx_wake_queue and thus eventually free_old_xmit_skbs are called.
> > It might make more sense to just wake the device without going through
> > an interrupt.
>
> I'm not sure I get this. If we don't enable tx napi, we tend to delay TX
> interrupt if we found the ring is about to full to avoid interrupt
> storm, so we're probably ok in this case.
I'm only concerned about the transition state when converting from
napi to no-napi when the queue is stopped and tx interrupt disabled.
With napi mode the interrupt is only disabled if napi is scheduled,
in which case it will eventually reenable the interrupt. But when
switching to no-napi mode in this state no progress will be made.
But it seems this cannot happen. When converting to no-napi
mode, set_coalesce waits for napi to complete in napi_disable.
So the interrupt should always start enabled when transitioning
into no-napi mode.
^ permalink raw reply
* [PATCH 0/2] net: ethernet: neterion: use linux/io-64-nonatomic-lo-hi.h
From: Corentin Labbe @ 2018-09-14 10:33 UTC (permalink / raw)
To: davem, jdmason; +Cc: linux-kernel, netdev, Corentin Labbe
Hello
This serie remove usage of custom writeq/readq in favor of ones defined in linux/io-64-nonatomic-lo-hi.h
This serie is only compile tested.
Regards
Corentin Labbe (2):
net: neterion: vxge: use linux/io-64-nonatomic-lo-hi.h
net: neterion: s2io: Use linux/io-64-nonatomic-lo-hi.h
drivers/net/ethernet/neterion/s2io.c | 1 +
drivers/net/ethernet/neterion/s2io.h | 22 +---------------------
drivers/net/ethernet/neterion/vxge/vxge-config.c | 1 +
drivers/net/ethernet/neterion/vxge/vxge-config.h | 20 --------------------
drivers/net/ethernet/neterion/vxge/vxge-traffic.c | 1 +
5 files changed, 4 insertions(+), 41 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Ard Biesheuvel @ 2018-09-14 6:21 UTC (permalink / raw)
To: Eric Biggers
Cc: Jason A. Donenfeld, LKML, Netdev, David Miller,
Greg Kroah-Hartman, Andrew Lutomirski, Samuel Neves,
Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <20180912183444.GB222557@gmail.com>
On 12 September 2018 at 20:34, Eric Biggers <ebiggers@kernel.org> wrote:
> On Wed, Sep 12, 2018 at 08:19:21PM +0200, Ard Biesheuvel wrote:
>> On 12 September 2018 at 20:16, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>> > Hi Eric,
>> >
>> > On Wed, Sep 12, 2018 at 12:08 AM Eric Biggers <ebiggers@kernel.org> wrote:
>> >> I'd strongly prefer the assembly to be readable too. Jason, I'm not sure if
>> >> you've actually read through the asm from the OpenSSL implementations, but the
>> >> generated .S files actually do lose a lot of semantic information that was in
>> >> the original .pl scripts.
>> >
>> > The thing to keep in mind is that the .S was not directly and blindly
>> > generated from the .pl. We started with the output of the .pl, and
>> > then, particularly in the case of x86_64, worked with it a lot, and
>> > now it's something a bit different. We've definitely spent a lot of
>> > time reading that assembly.
>> >
>>
>> Can we please have those changes as a separate patch? Preferably to
>> the .pl file rather than the .S file, so we can easily distinguish the
>> code from upstream from the code that you modified.
>>
>> > I'll see if I can improve the readability with some register name
>> > remapping on ARM. No guarantees, but I'll play a bit and see if I can
>> > make it a bit better.
>> >
>> > Jason
>
> FWIW, yesterday I made a modified version of poly1305-armv4.pl that generates an
> asm file that works in kernel mode. The changes are actually pretty small, and
> I think we can get them upstream into OpenSSL like they were for sha256-armv4.pl
> and sha512-armv4.pl. I'll start a thread with Andy Polyakov and you two.
>
> But I don't have time to help with all the many OpenSSL asm files Jason is
> proposing, just maybe poly1305-armv4 and chacha-armv4 for now.
>
Thanks Eric. I reached out to Andy Polyakov off line, and he is happy
to work with us again on this, although he did point out that our
experiences on ARM may not extrapolate to x86_64, given the fact that
the perl sources there also contain parameterization for the calling
convention differences between Windows and SysV.
^ permalink raw reply
* [PATCH net] net: diag: Fix swapped src/dst in udp_dump_one.
From: Lorenzo Colitti @ 2018-09-14 6:25 UTC (permalink / raw)
To: netdev; +Cc: davem, zenczykowski, dsahern, jeffv, Lorenzo Colitti
Since its inception, udp_dump_one had has a bug where userspace
needs to swap src and dst addresses and ports in order to find
the socket it wants.
This is because udp_dump_one misuses __udp[46]_lib_lookup by
passing the source address as the source address argument.
Unfortunately, those functions are intended to find local sockets
matching received packets, so the order of the arguments is
inverted: the argument that ends up being compared with, e.g.,
sk_daddr is actually saddr, not daddr.
While it's true that this creates a backwards compatibility
problem, this is clearly a bug since inet_diag_sockid is very
clear about which struct elements are the source address and port
and which are the destination address and port. Also, this bug
does not affect TCP sockets, SOCK_DESTROY of UDP sockets, or
finding UDP sockets with NLMSG_DUMP.
Fixes: a925aa00a55 ("udp_diag: Implement the get_exact dumping functionality")
Tested: https://android-review.googlesource.com/c/kernel/tests/+/755889/
Signed-off-by: Lorenzo Colitti <lorenzo@google.com>
---
net/ipv4/udp_diag.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/udp_diag.c b/net/ipv4/udp_diag.c
index d9ad986c7b..e1c6f90a92 100644
--- a/net/ipv4/udp_diag.c
+++ b/net/ipv4/udp_diag.c
@@ -43,16 +43,16 @@ static int udp_dump_one(struct udp_table *tbl, struct sk_buff *in_skb,
rcu_read_lock();
if (req->sdiag_family == AF_INET)
sk = __udp4_lib_lookup(net,
- req->id.idiag_src[0], req->id.idiag_sport,
req->id.idiag_dst[0], req->id.idiag_dport,
+ req->id.idiag_src[0], req->id.idiag_sport,
req->id.idiag_if, 0, tbl, NULL);
#if IS_ENABLED(CONFIG_IPV6)
else if (req->sdiag_family == AF_INET6)
sk = __udp6_lib_lookup(net,
- (struct in6_addr *)req->id.idiag_src,
- req->id.idiag_sport,
(struct in6_addr *)req->id.idiag_dst,
req->id.idiag_dport,
+ (struct in6_addr *)req->id.idiag_src,
+ req->id.idiag_sport,
req->id.idiag_if, 0, tbl, NULL);
#endif
if (sk && !refcount_inc_not_zero(&sk->sk_refcnt))
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* Re: Regression: kernel 4.14 an later very slow with many ipsec tunnels
From: Wolfgang Walter @ 2018-09-14 11:49 UTC (permalink / raw)
To: Florian Westphal
Cc: Steffen Klassert, David Miller, netdev, linux-kernel, torvalds,
christophe.gouault
In-Reply-To: <20180914055437.77pffp2jrbfnykbp@breakpoint.cc>
Am Freitag, 14. September 2018, 07:54:37 schrieb Florian Westphal:
> Steffen Klassert <steffen.klassert@secunet.com> wrote:
> > On Thu, Sep 13, 2018 at 11:03:25PM +0200, Florian Westphal wrote:
> > > David Miller <davem@davemloft.net> wrote:
> > > > From: Florian Westphal <fw@strlen.de>
> > > > Date: Thu, 13 Sep 2018 18:38:48 +0200
> > > >
> > > > > Wolfgang Walter <linux@stwm.de> wrote:
> > > > >> What I can say is that it depends mainly on number of policy rules
> > > > >> and SA.
> > > > >
> > > > > Thats already a good hint, I guess we're hitting long hash chains in
> > > > > xfrm_policy_lookup_bytype().
> > > >
> > > > I don't really see how recent changes can influence that.
> > >
> > > I don't think there is a recent change that did this.
> > >
> > > Walter says < 4.14 is ok, so this is likely related to flow cache
> > > removal.
> > >
> > > F.e. it looks like all prefixed policies end up in a linked list
> > > (net->xfrm.policy_inexact) and are not even in a hash table.
> > >
> > > I am staring at b58555f1767c9f4e330fcf168e4e753d2d9196e0
> > > but can't figure out how to configure that away from the
> > > 'no hashing for prefixed policies' default or why we even have
> > > policy_inexact in first place :/
> >
> > The hash threshold can be configured like this:
> >
> > ip x p set hthresh4 0 0
> >
> > This sets the hash threshold to local /0 and remote /0 netmasks.
> > With this configuration, all policies should go to the hashtable.
>
> Yes, but won't they all be hashed to same bucket?
>
> [ jhash(addr & 0, addr & 0) ] ?
>
> > Default hash thresholds are local /32 and remote /32 netmasks, so
> > all prefixed policies go to the inexact list.
>
> Yes.
>
> Wolfgang, before having to work on getting perf into your router image
> can you perhaps share a bit of info about the policies you're using?
>
> How many are there? Are they prefixed or not ("10.1.2.1")?
All rules are tunnel rules. That is they are rules like (in strongswan
notation)
conn A-to-B
left=111.111.111.111
leftsubnet=10.148.32.0/24
leftsigkey=....
right=111.111.111.222
rightsubnet=10.148.13.224/29
rightsigkey=....
esp=aes128ctr-sha1-ecp256-esn!
ike=aes128ctr-sha1-ecp256!
mobike=no
type=tunnel
....
(... other options not important here).
leftsubnet and rightsubnet may have any prefix from /30 to /16 here (we do not
yet use ipv6 but will do so next year).
We have about 3000 of them.
strongswan install IN, FWD and OUT rules for that in the kernel security
policy database with automated generated priorities (and SAs are generated
when strongswan actually establish a tunnel).
Also some of the rules overlap in range, that means ordering is important.
With IKEv2 this may happens automatically for SAs even if you avoid it in your
rule set as IKEv2 allows narrowing.
In policies you most often get this if you want to excempt a certain network
or host. We have a about 70 of them at the moment.
We do not use other possible selectors beside src-addr-range and dst-addr-
range (you could additionally select by protocol (icmp, udp, tcp), src- and
dst-port-range). So theoretically you could have a ruleset where there is a
rule with exempts all connection to dst port 22 for several network or applies
different encryption options and so on.
A rule determins what has to be done with the packet (sending or receiving)
from an ipsec-point of view: allow it without ipsec-transformation, block it
completely, or require certain ipsec transformation (use this or that
ecnryption scheme, use header compression, use transport or tunnel mode, ...)
So for any packet the kernel sends it has to look up if there are SAs which
matches and from these chose that with the highest priority (which is that one
with the lowest priority field). If there is none he has to lookup if there is
a matching policy, again choosing the one with the highest priority (and then
let the IKE-daemon actually establish a SA). For tunnel-mode he actually has
to do it twice, I think, as the tunnel-paket again passes ipsec.
For every packet it receives and which ist not an ipsec paket he has to do a
lookup in the policy database to see if it should have been (or if it is
allowed or blocked). If no rule is found it is allowed without encryption. We
have 29.000 allow rules. I did deactivate them for the tests with 4.14 and
4.18 as these makes things horrible. They are automatically generated from our
declarativ network description and we actually don't need them as they do not
overlap with the remote networks tunneled via ipsec. They did not impose any
burden for 4.9 and earlier.
We sometimes need them (say if 10.10.0.0/16 is remote but 10.10.1.0 which is
local).
So this is basically the multidimensional packet classifiction problem: from a
set of m-dimensional blocks find that one with the highest priority which
contains a certain point.
The dimension here are src-addr-range, dst-addr-range, protocol, src-port-
range, dst-port-range.
If your rule is itself a point you may hash it (and you can only do this if it
is sure that there is no other non-point rule with higher prio matching this
point rule as there is no such rule that a more specific rule beats a less
specific rule (this would be ill defined)).
Here an example how strongswan allows you to use all of the above selectors
for your rules. For example you could write for leftsubnet:
leftsubnet=10.0.0.1[tcp/http],10.0.0.2[6/80]
leftsubnet=fec1::1[udp],10.0.0.0/16[/53].
leftsubnet=fec1::1[udp/%any],10.0.0.0/16[%any/53]
leftsubnet=fec1::1[udp/%any],10.0.0.0/16[%any/1024-32000]
So ipsec with large policy-database without xfrm flow cache is comparable with
a large netfilter ruleset (with only one chain) without conntrack.
Regards,
--
Wolfgang Walter
Studentenwerk München
Anstalt des öffentlichen Rechts
^ permalink raw reply
* Re: mlx5 driver loading failing on v4.19 / net-next / bpf-next
From: Jesper Dangaard Brouer @ 2018-09-14 6:36 UTC (permalink / raw)
To: Alexei Starovoitov, Moshe Shemesh, Eli Cohen, Or Gerlitz
Cc: Tariq Toukan, Saeed Mahameed, netdev@vger.kernel.org,
Eran Ben Elisha, brouer
In-Reply-To: <CAADnVQJR0NA-eBcPiEnUKoswhOZ7VKp+MNr3qg+b7gSXaw7+zw@mail.gmail.com>
On Thu, 13 Sep 2018 15:55:29 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 30, 2018 at 1:35 AM, Tariq Toukan <tariqt@mellanox.com> wrote:
> >
> >
> > On 29/08/2018 6:05 PM, Jesper Dangaard Brouer wrote:
> >>
> >> Hi Saeed,
> >>
> >> I'm having issues loading mlx5 driver on v4.19 kernels (tested both
> >> net-next and bpf-next), while kernel v4.18 seems to work. It happens
> >> with a Mellanox ConnectX-5 NIC (and also a CX4-Lx but I removed that
> >> from the system now).
> >>
> >
> > Hi Jesper,
> >
> > Thanks for your report!
> >
> > We are working to analyze and debug the issue.
>
> looks like serious issue to me... while no news in 2 weeks.
> any update?
Mellanox took it offlist, and Sep 6th found that this is a regression
introduced by commit 269d26f47f6f ("net/mlx5: Reduce command polling
interval"), but only if CONFIG_PREEMPT is on.
I can confirm that reverting this commit fixed the issue (and not the
firmware upgrade I also did).
I think Moshe (Cc) is responsible for this case, and I expect to soon
see a revert or alternative solution to this!?
Thanks for the kick Alexei :-)
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox