* [PATCH net-next v2 09/12] net: sched: flower: handle concurrent tcf proto deletion
From: Vlad Buslov @ 2019-02-27 10:12 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, sbrivio, Vlad Buslov
In-Reply-To: <20190227101226.26196-1-vladbu@mellanox.com>
Without rtnl lock protection tcf proto can be deleted concurrently. Check
tcf proto 'deleting' flag after taking tcf spinlock to verify that no
concurrent deletion is in progress. Return EAGAIN error if concurrent
deletion detected, which will cause caller to retry and possibly create new
instance of tcf proto.
Retry mechanism is a result of fine-grained locking approach used in this
and previous changes in series and is necessary to allow concurrent updates
on same chain instance. Alternative approach would be to lock the whole
chain while updating filters on any of child tp's, adding and removing
classifier instances from the chain. However, since most CPU-intensive
parts of filter update code are specifically in classifier code and its
dependencies (extensions and hw offloads), such approach would negate most
of the gains introduced by this change and previous changes in the series
when updating same chain instance.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
Changes from V1 to V2:
- Extend commit message.
- Fix error code in comment.
net/sched/cls_flower.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 70b357f23391..25a4d64b82db 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -1500,6 +1500,14 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
if (!tc_in_hw(fnew->flags))
fnew->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
+ /* tp was deleted concurrently. -EAGAIN will cause caller to lookup
+ * proto again or create new one, if necessary.
+ */
+ if (tp->deleting) {
+ err = -EAGAIN;
+ goto errout_hw;
+ }
+
refcount_inc(&fnew->refcnt);
if (fold) {
/* Fold filter was deleted concurrently. Retry lookup. */
--
2.13.6
^ permalink raw reply related
* [PATCH net-next v2 05/12] net: sched: flower: add reference counter to flower mask
From: Vlad Buslov @ 2019-02-27 10:12 UTC (permalink / raw)
To: netdev; +Cc: jhs, xiyou.wangcong, jiri, davem, sbrivio, Vlad Buslov
In-Reply-To: <20190227101226.26196-1-vladbu@mellanox.com>
Extend fl_flow_mask structure with reference counter to allow parallel
modification without relying on rtnl lock. Use rcu read lock to safely
lookup mask and increment reference counter in order to accommodate
concurrent deletes.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Acked-by: Jiri Pirko <jiri@mellanox.com>
---
net/sched/cls_flower.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index dd8a65cef6e1..e98313cd710a 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -76,6 +76,7 @@ struct fl_flow_mask {
struct list_head filters;
struct rcu_work rwork;
struct list_head list;
+ refcount_t refcnt;
};
struct fl_flow_tmplt {
@@ -320,6 +321,7 @@ static int fl_init(struct tcf_proto *tp)
static void fl_mask_free(struct fl_flow_mask *mask)
{
+ WARN_ON(!list_empty(&mask->filters));
rhashtable_destroy(&mask->ht);
kfree(mask);
}
@@ -335,7 +337,7 @@ static void fl_mask_free_work(struct work_struct *work)
static bool fl_mask_put(struct cls_fl_head *head, struct fl_flow_mask *mask,
bool async)
{
- if (!list_empty(&mask->filters))
+ if (!refcount_dec_and_test(&mask->refcnt))
return false;
rhashtable_remove_fast(&head->ht, &mask->ht_node, mask_ht_params);
@@ -1301,6 +1303,7 @@ static struct fl_flow_mask *fl_create_new_mask(struct cls_fl_head *head,
INIT_LIST_HEAD_RCU(&newmask->filters);
+ refcount_set(&newmask->refcnt, 1);
err = rhashtable_insert_fast(&head->ht, &newmask->ht_node,
mask_ht_params);
if (err)
@@ -1324,9 +1327,13 @@ static int fl_check_assign_mask(struct cls_fl_head *head,
struct fl_flow_mask *mask)
{
struct fl_flow_mask *newmask;
+ int ret = 0;
+ rcu_read_lock();
fnew->mask = rhashtable_lookup_fast(&head->ht, mask, mask_ht_params);
if (!fnew->mask) {
+ rcu_read_unlock();
+
if (fold)
return -EINVAL;
@@ -1335,11 +1342,15 @@ static int fl_check_assign_mask(struct cls_fl_head *head,
return PTR_ERR(newmask);
fnew->mask = newmask;
+ return 0;
} else if (fold && fold->mask != fnew->mask) {
- return -EINVAL;
+ ret = -EINVAL;
+ } else if (!refcount_inc_not_zero(&fnew->mask->refcnt)) {
+ /* Mask was deleted concurrently, try again */
+ ret = -EAGAIN;
}
-
- return 0;
+ rcu_read_unlock();
+ return ret;
}
static int fl_set_parms(struct net *net, struct tcf_proto *tp,
@@ -1476,6 +1487,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
list_replace_rcu(&fold->list, &fnew->list);
fold->deleted = true;
+ fl_mask_put(head, fold->mask, true);
if (!tc_skip_hw(fold->flags))
fl_hw_destroy_filter(tp, fold, NULL);
tcf_unbind_filter(tp, &fold->res);
@@ -1525,7 +1537,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
if (!tc_skip_hw(fnew->flags))
fl_hw_destroy_filter(tp, fnew, NULL);
errout_mask:
- fl_mask_put(head, fnew->mask, false);
+ fl_mask_put(head, fnew->mask, true);
errout:
tcf_exts_destroy(&fnew->exts);
kfree(fnew);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH 0/4] mwifiex PCI/wake-up interrupt fixes
From: Ard Biesheuvel @ 2019-02-27 10:16 UTC (permalink / raw)
To: Marc Zyngier
Cc: Brian Norris, Ganapathi Bhat, Jeffy Chen, Heiko Stuebner,
Devicetree List, Xinming Hu, <netdev@vger.kernel.org>,
linux-pm, <linux-wireless@vger.kernel.org>,
Linux Kernel Mailing List, Amitkumar Karwar, linux-rockchip,
Nishant Sarmukadam, Rob Herring, Rafael J. Wysocki,
linux-arm-kernel, Enric Balletbo i Serra, Lorenzo Pieralisi,
David S. Miller, Kalle Valo, Tony Lindgren
In-Reply-To: <d67512fe-42b4-513f-d27a-fed85c19e9c2@arm.com>
On Wed, 27 Feb 2019 at 11:02, Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> + Lorenzo
>
> Hi Brian,
>
> On 26/02/2019 23:28, Brian Norris wrote:
> > + others
> >
> > Hi Marc,
> >
> > Thanks for the series. I have a few bits of history to add to this, and
> > some comments.
> >
> > On Sun, Feb 24, 2019 at 02:04:22PM +0000, Marc Zyngier wrote:
> >> For quite some time, I wondered why the PCI mwifiex device built in my
> >> Chromebook was unable to use the good old legacy interrupts. But as MSIs
> >> were working fine, I never really bothered investigating. I finally had a
> >> look, and the result isn't very pretty.
> >>
> >> On this machine (rk3399-based kevin), the wake-up interrupt is described as
> >> such:
> >>
> >> &pci_rootport {
> >> mvl_wifi: wifi@0,0 {
> >> compatible = "pci1b4b,2b42";
> >> reg = <0x83010000 0x0 0x00000000 0x0 0x00100000
> >> 0x83010000 0x0 0x00100000 0x0 0x00100000>;
> >> interrupt-parent = <&gpio0>;
> >> interrupts = <8 IRQ_TYPE_LEVEL_LOW>;
> >> pinctrl-names = "default";
> >> pinctrl-0 = <&wlan_host_wake_l>;
> >> wakeup-source;
> >> };
> >> };
> >>
> >> Note how the interrupt is part of the properties directly attached to the
> >> PCI node. And yet, this interrupt has nothing to do with a PCI legacy
> >> interrupt, as it is attached to the wake-up widget that bypasses the PCIe RC
> >> altogether (Yay for the broken design!). This is in total violation of the
> >> IEEE Std 1275-1994 spec[1], which clearly documents that such interrupt
> >> specifiers describe the PCI device interrupts, and must obey the
> >> INT-{A,B,C,D} mapping. Oops!
> >
> > You're not the first person to notice this. All the motivations are not
> > necessarily painted clearly in their cover letter, but here are some
> > previous attempts at solving this problem:
> >
> > [RFC PATCH v11 0/5] PCI: rockchip: Move PCIe WAKE# handling into pci core
> > https://lkml.kernel.org/lkml/20171225114742.18920-1-jeffy.chen@rock-chips.com/
> > http://lkml.kernel.org/lkml/20171226023646.17722-1-jeffy.chen@rock-chips.com/
> >
> > As you can see by the 12th iteration, it wasn't left unsolved for lack
> > of trying...
>
> I wasn't aware of this. That's definitely a better approach than my
> hack, and I would really like this to be revived.
>
I don't think this approach is entirely sound either.
From the side of the PCI device, WAKE# is just a GPIO line, and how it
is wired into the system is an entirely separate matter. So I don't
think it is justified to overload the notion of legacy interrupts with
some other pin that may behave in a way that is vaguely similar to how
a true wake-up capable interrupt works.
So I'd argue that we should add an optional 'wake-gpio' DT property
instead to the generic PCI device binding, and leave the interrupt
binding and discovery alone.
^ permalink raw reply
* RE: Realtek r8822be kernel module does not negotiate 802.11ac connection
From: Tony Chuang @ 2019-02-27 10:23 UTC (permalink / raw)
To: David R. Bergstein
Cc: Pkshih, linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <51d62a31-7cde-e815-bdee-4ef5433007aa@gmail.com>
> This message is in regard to a bug I have open on bugs.launchpad.net, 1813372,
> linked below. This issue, originally identified in an Ubuntu kernel, has been
> duplicated in the most current mainline kernel, 5.0-rc8, and is in regard to
> problems attaining a wireless connection at 802.11ac speeds.
>
> https://bugs.launchpad.net/bugs/1813372
>
> At your earliest convenience, please see the bug report above, and advise if a fix
> will be available for the r8822be kernel module.
>
> Sincerely,
>
> David R. Bergstein
>
Hi,
If it's possible I will suggest that you can use the new driver "rtw88" being
reviewed now. rtw88 is better supported by Realtek and has significant
improvement in contrast to r8822be. And also it is aim to better support
802.11ac series ICs, so it may help resolve your problem about 802.11ac
connection speeds.
Thanks!
Yan-Hsuan
^ permalink raw reply
* [PATCH v1] net: dev: Use unsigned integer as an argument to left-shift
From: Andy Shevchenko @ 2019-02-27 10:37 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Andy Shevchenko
1 << 31 is Undefined Behaviour according to the C standard.
Use U type modifier to avoid theoretical overflow.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
include/linux/netdevice.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 86dbb3e29139..848b54b7ec91 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3861,7 +3861,7 @@ static inline u32 netif_msg_init(int debug_value, int default_msg_enable_bits)
if (debug_value == 0) /* no output */
return 0;
/* set low N bits */
- return (1 << debug_value) - 1;
+ return (1U << debug_value) - 1;
}
static inline void __netif_tx_lock(struct netdev_queue *txq, int cpu)
--
2.20.1
^ permalink raw reply related
* [PATCH v1] enc28j60: Correct description of debug module parameter
From: Andy Shevchenko @ 2019-02-27 10:45 UTC (permalink / raw)
To: David S. Miller, netdev; +Cc: Andy Shevchenko
The netif_msg_init() API takes on input the amount of bits to be set. The
description of debug parameter in the enc28j60 module is misleading in this
sense and passing 0xffff does not give an expected behaviour.
Fix the description of debug module parameter to show what exactly is expected.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/net/ethernet/microchip/enc28j60.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/microchip/enc28j60.c b/drivers/net/ethernet/microchip/enc28j60.c
index f6ecfa778660..8f72587b5a2c 100644
--- a/drivers/net/ethernet/microchip/enc28j60.c
+++ b/drivers/net/ethernet/microchip/enc28j60.c
@@ -1681,5 +1681,5 @@ MODULE_DESCRIPTION(DRV_NAME " ethernet driver");
MODULE_AUTHOR("Claudio Lanconelli <lanconelli.claudio@eptar.com>");
MODULE_LICENSE("GPL");
module_param_named(debug, debug.msg_enable, int, 0);
-MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., ffff=all)");
+MODULE_PARM_DESC(debug, "Debug verbosity level in amount of bits set (0=none, ..., 31=all)");
MODULE_ALIAS("spi:" DRV_NAME);
--
2.20.1
^ permalink raw reply related
* Re: [PATCH V4] can: flexcan: implement can Runtime PM
From: Marc Kleine-Budde @ 2019-02-27 10:35 UTC (permalink / raw)
To: Joakim Zhang, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx, Aisheng DONG
In-Reply-To: <20181130085119.7948-1-qiangqing.zhang@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 10064 bytes --]
On 11/30/18 9:53 AM, Joakim Zhang wrote:
> From: Aisheng Dong <aisheng.dong@nxp.com>
>
> Flexcan will be disabled during suspend if no wakeup function required and
> enabled after resume accordingly. During this period, we could explicitly
> disable clocks.
> Since PM is optional, the clock is enabled at probe to guarante the
> clock is running when PM is not enabled in the kernel.
>
> Implement Runtime PM which will:
> 1) Without CONFIG_PM, clock is running whether Flexcan up or down.
> 2) With CONFIG_PM, clock enabled while Flexcan up and disabled when
> Flexcan down.
> 3) Disable clock when do system suspend and enable clock while system
> resume.
> 4) Make Power Domain framework be able to shutdown the corresponding
> power domain of this device.
>
> Signed-off-by: Aisheng Dong <aisheng.dong@nxp.com>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> ChangeLog:
> V1->V2:
> *rebased on patch "can: flexcan: add self wakeup support".
> V2->V3:
> *fix device fails to probe without CONFIG_PM.
> V3->V4:
> *runtime pm enable should ahead of registering device.
> *disable device even if keeping the clocks on.
> ---
> drivers/net/can/flexcan.c | 111 +++++++++++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index 0f36eafe3ac1..cad42f20cfe5 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -24,6 +24,7 @@
> #include <linux/of.h>
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> #include <linux/regmap.h>
>
> @@ -277,6 +278,7 @@ struct flexcan_priv {
> u32 reg_imask1_default;
> u32 reg_imask2_default;
>
> + struct device *dev;
> struct clk *clk_ipg;
> struct clk *clk_per;
> const struct flexcan_devtype_data *devtype_data;
> @@ -444,6 +446,27 @@ static inline void flexcan_error_irq_disable(const struct flexcan_priv *priv)
> priv->write(reg_ctrl, ®s->ctrl);
> }
>
> +static int flexcan_clks_enable(const struct flexcan_priv *priv)
> +{
> + int err;
> +
> + err = clk_prepare_enable(priv->clk_ipg);
> + if (err)
> + return err;
> +
> + err = clk_prepare_enable(priv->clk_per);
> + if (err)
> + clk_disable_unprepare(priv->clk_ipg);
> +
> + return err;
> +}
> +
> +static void flexcan_clks_disable(const struct flexcan_priv *priv)
> +{
> + clk_disable_unprepare(priv->clk_ipg);
> + clk_disable_unprepare(priv->clk_per);
The original code first disabled the per then the ipg.
> +}
> +
> static inline int flexcan_transceiver_enable(const struct flexcan_priv *priv)
> {
> if (!priv->reg_xceiver)
> @@ -570,19 +593,13 @@ static int flexcan_get_berr_counter(const struct net_device *dev,
> const struct flexcan_priv *priv = netdev_priv(dev);
> int err;
>
> - err = clk_prepare_enable(priv->clk_ipg);
> - if (err)
> + err = pm_runtime_get_sync(priv->dev);
> + if (err < 0)
> return err;
>
> - err = clk_prepare_enable(priv->clk_per);
> - if (err)
> - goto out_disable_ipg;
> -
> err = __flexcan_get_berr_counter(dev, bec);
>
> - clk_disable_unprepare(priv->clk_per);
> - out_disable_ipg:
> - clk_disable_unprepare(priv->clk_ipg);
> + pm_runtime_put(priv->dev);
>
> return err;
> }
> @@ -1215,17 +1232,13 @@ static int flexcan_open(struct net_device *dev)
> struct flexcan_priv *priv = netdev_priv(dev);
> int err;
>
> - err = clk_prepare_enable(priv->clk_ipg);
> - if (err)
> + err = pm_runtime_get_sync(priv->dev);
> + if (err < 0)
> return err;
>
> - err = clk_prepare_enable(priv->clk_per);
> - if (err)
> - goto out_disable_ipg;
> -
> err = open_candev(dev);
> if (err)
> - goto out_disable_per;
> + goto out_disable_clks;
nitpick: you do a pm_runtime_put, so rename the label...
>
> err = request_irq(dev->irq, flexcan_irq, IRQF_SHARED, dev->name, dev);
> if (err)
> @@ -1288,10 +1301,8 @@ static int flexcan_open(struct net_device *dev)
> free_irq(dev->irq, dev);
> out_close:
> close_candev(dev);
> - out_disable_per:
> - clk_disable_unprepare(priv->clk_per);
> - out_disable_ipg:
> - clk_disable_unprepare(priv->clk_ipg);
> + out_disable_clks:
> + pm_runtime_put(priv->dev);
...to out_runtime_put
>
> return err;
> }
> @@ -1306,10 +1317,9 @@ static int flexcan_close(struct net_device *dev)
>
> can_rx_offload_del(&priv->offload);
> free_irq(dev->irq, dev);
> - clk_disable_unprepare(priv->clk_per);
> - clk_disable_unprepare(priv->clk_ipg);
>
> close_candev(dev);
> + pm_runtime_put(priv->dev);
>
> can_led_event(dev, CAN_LED_EVENT_STOP);
>
> @@ -1349,18 +1359,14 @@ static int register_flexcandev(struct net_device *dev)
> struct flexcan_regs __iomem *regs = priv->regs;
> u32 reg, err;
>
> - err = clk_prepare_enable(priv->clk_ipg);
> + err = flexcan_clks_enable(priv);
> if (err)
> return err;
>
> - err = clk_prepare_enable(priv->clk_per);
> - if (err)
> - goto out_disable_ipg;
> -
> /* select "bus clock", chip must be disabled */
> err = flexcan_chip_disable(priv);
> if (err)
> - goto out_disable_per;
> + goto out_disable_clks;
nitpick: The function is called lexcan_clks_disable(), so let's rename
the label accordingly.
> reg = priv->read(®s->ctrl);
> reg |= FLEXCAN_CTRL_CLK_SRC;
> priv->write(reg, ®s->ctrl);
> @@ -1389,14 +1395,13 @@ static int register_flexcandev(struct net_device *dev)
>
> err = register_candev(dev);
No error handling?
>
> - /* disable core and turn off clocks */
Better move the pm_runtime_put() here and adjust the comment.
> - out_chip_disable:
> flexcan_chip_disable(priv);
> - out_disable_per:
> - clk_disable_unprepare(priv->clk_per);
> - out_disable_ipg:
> - clk_disable_unprepare(priv->clk_ipg);
> + return 0;
>
> + out_chip_disable:
> + flexcan_chip_disable(priv);
> + out_disable_clks:
> + flexcan_clks_disable(priv);
> return err;
> }
>
> @@ -1556,6 +1561,7 @@ static int flexcan_probe(struct platform_device *pdev)
> priv->write = flexcan_write_le;
> }
>
> + priv->dev = &pdev->dev;
> priv->can.clock.freq = clock_freq;
> priv->can.bittiming_const = &flexcan_bittiming_const;
> priv->can.do_set_mode = flexcan_set_mode;
> @@ -1569,6 +1575,10 @@ static int flexcan_probe(struct platform_device *pdev)
> priv->devtype_data = devtype_data;
> priv->reg_xceiver = reg_xceiver;
>
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_set_active(&pdev->dev);
> + pm_runtime_enable(&pdev->dev);
> +
> err = register_flexcandev(dev);
> if (err) {
> dev_err(&pdev->dev, "registering netdev failed\n");
> @@ -1586,6 +1596,7 @@ static int flexcan_probe(struct platform_device *pdev)
> dev_info(&pdev->dev, "device registered (reg_base=%p, irq=%d)\n",
> priv->regs, dev->irq);
>
> + pm_runtime_put(&pdev->dev);
> return 0;
>
> failed_register:
> @@ -1598,6 +1609,7 @@ static int flexcan_remove(struct platform_device *pdev)
> struct net_device *dev = platform_get_drvdata(pdev);
>
> unregister_flexcandev(dev);
> + pm_runtime_disable(&pdev->dev);
> free_candev(dev);
>
> return 0;
> @@ -1607,7 +1619,7 @@ static int __maybe_unused flexcan_suspend(struct device *device)
> {
> struct net_device *dev = dev_get_drvdata(device);
> struct flexcan_priv *priv = netdev_priv(dev);
> - int err;
> + int err = 0;
>
> if (netif_running(dev)) {
> /* if wakeup is enabled, enter stop mode
> @@ -1620,20 +1632,22 @@ static int __maybe_unused flexcan_suspend(struct device *device)
> err = flexcan_chip_disable(priv);
> if (err)
> return err;
> +
> + err = pm_runtime_force_suspend(device);
> }
> netif_stop_queue(dev);
> netif_device_detach(dev);
> }
> priv->can.state = CAN_STATE_SLEEPING;
>
> - return 0;
> + return err;
> }
>
> static int __maybe_unused flexcan_resume(struct device *device)
> {
> struct net_device *dev = dev_get_drvdata(device);
> struct flexcan_priv *priv = netdev_priv(dev);
> - int err;
> + int err = 0;
>
> priv->can.state = CAN_STATE_ERROR_ACTIVE;
> if (netif_running(dev)) {
> @@ -1642,14 +1656,34 @@ static int __maybe_unused flexcan_resume(struct device *device)
> if (device_may_wakeup(device)) {
> disable_irq_wake(dev->irq);
> } else {
> - err = flexcan_chip_enable(priv);
> + err = pm_runtime_force_resume(device);
> if (err)
> return err;
> +
> + err = flexcan_chip_enable(priv);
> }
> }
> + return err;
> +}
> +
> +static int __maybe_unused flexcan_runtime_suspend(struct device *device)
> +{
> + struct net_device *dev = dev_get_drvdata(device);
> + struct flexcan_priv *priv = netdev_priv(dev);
> +
> + flexcan_clks_disable(priv);
> +
> return 0;
> }
>
> +static int __maybe_unused flexcan_runtime_resume(struct device *device)
> +{
> + struct net_device *dev = dev_get_drvdata(device);
> + struct flexcan_priv *priv = netdev_priv(dev);
> +
> + return flexcan_clks_enable(priv);
> +}
> +
> static int __maybe_unused flexcan_noirq_suspend(struct device *device)
> {
> struct net_device *dev = dev_get_drvdata(device);
> @@ -1676,6 +1710,7 @@ static int __maybe_unused flexcan_noirq_resume(struct device *device)
>
> static const struct dev_pm_ops flexcan_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(flexcan_suspend, flexcan_resume)
> + SET_RUNTIME_PM_OPS(flexcan_runtime_suspend, flexcan_runtime_resume, NULL)
> SET_NOIRQ_SYSTEM_SLEEP_PM_OPS(flexcan_noirq_suspend, flexcan_noirq_resume)
> };
>
>
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH] can: flexcan: fix timeout when set small bitrate
From: Marc Kleine-Budde @ 2019-02-27 11:06 UTC (permalink / raw)
To: Joakim Zhang, linux-can@vger.kernel.org
Cc: wg@grandegger.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, dl-linux-imx
In-Reply-To: <20190131093509.12613-1-qiangqing.zhang@nxp.com>
[-- Attachment #1.1: Type: text/plain, Size: 1250 bytes --]
On 1/31/19 10:37 AM, Joakim Zhang wrote:
> Current we can meet timeout issue when setting a small bitrate like 10000
> as follows on i.MX6UL EVK board (ipg clock = 66MHZ, per clock = 30MHZ):
> root@imx6ul7d:~# ip link set can0 up type can bitrate 10000
> A link change request failed with some changes committed already.
> Interface can0 may have been left with an inconsistent configuration, please check.
> RTNETLINK answers: Connection timed out
>
> It is caused by calling of flexcan_chip_unfreeze() timeout.
>
> Originally the code is using usleep_range(10, 20) for unfreeze operation,
> but the patch (8badd65 can: flexcan: avoid calling usleep_range from interrupt
> context) changed it into udelay(10) which is only a half delay of before,
> there're also some other delay changes.
>
> After double to FLEXCAN_TIMEOUT_US to 100 can fix the issue.
>
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
Applied to linux-can
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: stmmac / meson8b-dwmac
From: Jose Abreu @ 2019-02-27 11:09 UTC (permalink / raw)
To: Simon Huelck, Sebastian Gottschall, Jerome Brunet, Jose Abreu,
Martin Blumenstingl
Cc: linux-amlogic, Gpeppe.cavallaro, alexandre.torgue,
Emiliano Ingrassia, netdev
In-Reply-To: <3a040370-e7e5-990e-81dc-8e9bb0ab7761@gmx.de>
Hi Simon,
On 2/24/2019 8:34 PM, Simon Huelck wrote:
> the topic is about ODROID C2 / Amlogic S905X since the start. we have a
> performance regression since 4.14.
As we are not advancing in this topic I suggest you try bisecting
the offending commit.
Thanks,
Jose Miguel Abreu
^ permalink raw reply
* Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
From: Sheng Lan @ 2019-02-27 11:26 UTC (permalink / raw)
To: Eric Dumazet, Stephen Hemminger
Cc: davem, netdev, netem, xuhanbing, zhengshaoyu, jiqin.ji,
liuzhiqiang26, yuehaibing
In-Reply-To: <375ee6d3-ef86-c653-5d8d-df48cea8b3ba@gmail.com>
在 2019/2/26 23:52, Eric Dumazet 写道:
>
>
> On 02/26/2019 05:02 AM, Sheng Lan wrote:
>>
>>
>>
>>> On Mon, 25 Feb 2019 22:49:39 +0800
>>> Sheng Lan <lansheng@huawei.com> wrote:
>>>
>>>> From: Sheng Lan <lansheng@huawei.com>
>>>> Subject: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec
>>>>
>>>> It can be reproduced by following steps:
>>>> 1. virtio_net NIC is configured with gso/tso on
>>>> 2. configure nginx as http server with an index file bigger than 1M bytes
>>>> 3. use tc netem to produce duplicate packets and delay:
>>>> tc qdisc add dev eth0 root netem delay 100ms 10ms 30% duplicate 90%
>>>> 4. continually curl the nginx http server to get index file on client
>>>> 5. BUG_ON is seen quickly
>>>>
>>>> [10258690.371129] kernel BUG at net/core/skbuff.c:4028!
>>>> [10258690.371748] invalid opcode: 0000 [#1] SMP PTI
>>>> [10258690.372094] CPU: 5 PID: 0 Comm: swapper/5 Tainted: G W 5.0.0-rc6 #2
>>>> [10258690.372094] RSP: 0018:ffffa05797b43da0 EFLAGS: 00010202
>>>> [10258690.372094] RBP: 00000000000005ea R08: 0000000000000000 R09: 00000000000005ea
>>>> [10258690.372094] R10: ffffa0579334d800 R11: 00000000000002c0 R12: 0000000000000002
>>>> [10258690.372094] R13: 0000000000000000 R14: ffffa05793122900 R15: ffffa0578f7cb028
>>>> [10258690.372094] FS: 0000000000000000(0000) GS:ffffa05797b40000(0000) knlGS:0000000000000000
>>>> [10258690.372094] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>>> [10258690.372094] CR2: 00007f1a6dc00868 CR3: 000000001000e000 CR4: 00000000000006e0
>>>> [10258690.372094] Call Trace:
>>>> [10258690.372094] <IRQ>
>>>> [10258690.372094] skb_to_sgvec+0x11/0x40
>>>> [10258690.372094] start_xmit+0x38c/0x520 [virtio_net]
>>>> [10258690.372094] dev_hard_start_xmit+0x9b/0x200
>>>> [10258690.372094] sch_direct_xmit+0xff/0x260
>>>> [10258690.372094] __qdisc_run+0x15e/0x4e0
>>>> [10258690.372094] net_tx_action+0x137/0x210
>>>> [10258690.372094] __do_softirq+0xd6/0x2a9
>>>> [10258690.372094] irq_exit+0xde/0xf0
>>>> [10258690.372094] smp_apic_timer_interrupt+0x74/0x140
>>>> [10258690.372094] apic_timer_interrupt+0xf/0x20
>>>> [10258690.372094] </IRQ>
>>>>
>>>> In __skb_to_sgvec, the skb->len is not equal to the sum of the skb's
>>>> linear data size and nonlinear data size, thus BUG_ON triggered. The
>>>> bad skb's nonlinear data size is less than skb->data_len, because the
>>>> skb is cloned and a part of related cloned skb's nonlinear data is
>>>> split off.
>>>>
>>>> Duplicate packet is cloned by skb_clone in netem_enqueue and may be delayed
>>>> some time in qdisc. Due to the delay time, the original skb will be pushed
>>>> again later in __tcp_push_pending_frames when tcp receives new packets.
>>>> In tcp_write_xmit, when the tcp_mss_split_point returns a smaller limit,
>>>> the original skb will be fragmented and the skb's nonlinear data will be
>>>> split off. The length of the skb cloned by netem will not be updated.
>>>> When we use virtio_net NIC, the duplicated cloned skb will be filled into
>>>> a scatter-gather list in __skb_to_sgvec and trigger the BUG_ON.
>>>>
>>>> Here I replace the skb_clone with skb_copy in netem_enqueue to ensure
>>>> the duplicated skb's nonlinear data is independent.
>>>>
>>>> Signed-off-by: Sheng Lan <lansheng@huawei.com>
>>>> Reported-by: Qin Ji <jiqin.ji@huawei.com>
>>>>
>>>> Fixes: 0afb51e7 ("netem: reinsert for duplication")
>>>
>>> This sounds like a bug in the other layers (either TCP or Virtio net)
>>> not handling a cloned skb properly.
>>>
>>
>> I have traced the route of skb by printk, let me take an example to describe the problem to make it clearly:
>> Mss value equals to 1448. Limit value is the split size when tcp do tso_fragment, is depending on the size of the sending congestion window and mss value.
>>
>> TCP layer transmit the index file to client, the original skb1 size is large:
>> ...
>> tcp_write_xmit (skb1->data_len == 62264, limit == 2*mss == 2896)
>> tso_fragment (it needs to be fragmented by limit value)
>> skb_split (after split, skb1->data_len == 2896, skb_shinfo(skb1)->frags[0] == 2896, skb_shinfo(skb1)->nr_frags == 1)
>> ...
>> netem_enqueue (netem construct a duplicate packet of skb1 by skb_clone)
>> skb2 = skb_clone(skb1) (skb1->data_len == skb2->data_len == 2896, skb1 and skb2 share the nonlinear data frags[0] == 2896)
>> waiting 30ms (skb1 and skb2 will be delayed in qdisc queue due to the netem delay configuration)
>>
>>
>> TCP layer receives new packets and trys to retransmit the skb1:
>> tcp_rcv_established
>> __tcp_push_pending_frames
>> tcp_write_xmit (skb1->data_len == 2896, cwnd size decreased or packets in flight increased, cause the limit decreased to 1*mss == 1448)
>
> tcp_write_xmit() only deals with packet in the write queue,
> they never were sent. They can not be any clone of them by definition, since
> skbs in the TCP write queue are private to TCP stack,
>
> Once a packet is sent, the master skb is moved to the rtx rb-tree,
> while the clone is going through lower stacks.
>
> When/if a retransmit is due, we always make sure there is no clone on it,
> look at the various calls to skb_unclone()
I traced again and found that the skb was not sent, master skb was still in write queue,
because the function tcp_transmit_skb() returns 1 (NET_XMIT_DROP), thus it can be retransmit.
I found the error value NET_XMIT_DROP returns from netem_enqueue(), when the length of qdisc queue
is greater than queue limit value.
In netem_enqueue() the skb is cloned before returning the NET_XMIT_DROP error value,
thus the master skb is still in write queue and be cloned in netem_enqueue(). This may cause the master
skb be retransmit and fragmented again while it is cloned.
I think there are potential risks that tso_fragment() will get a cloned skb if skb is cloned by lower layer.
I try to fix it by moving returning error value statment to the front of the skb_clone() in netem_enqueue(), and it works.
And netem_enqueue() constructs corrupt packets statment returns NET_XMIT_DROP too. To fix this completely should I move the
constructing corrupt statment to the front of the skb_clone() ?
Please correct me if I am wrong, and I need your advice.
Thanks
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 75046ec..615a341 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -474,6 +474,9 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
if (q->latency || q->jitter || q->rate)
skb_orphan_partial(skb);
+ if (unlikely(sch->q.qlen >= sch->limit))
+ return qdisc_drop_all(skb, sch, to_free);
+
/*
* If we need to duplicate packet, then re-insert at top of the
* qdisc tree, since parent queuer expects that only one
@@ -521,9 +524,6 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch,
1<<(prandom_u32() % 8);
}
- if (unlikely(sch->q.qlen >= sch->limit))
- return qdisc_drop_all(skb, sch, to_free);
-
qdisc_qstats_backlog_inc(sch, skb);
cb = netem_skb_cb(skb);
--
^ permalink raw reply related
* Re: [PATCH iproute2-next] rdma: Add the prefix for driver attributes
From: Leon Romanovsky @ 2019-02-27 11:38 UTC (permalink / raw)
To: oulijun
Cc: David Ahern, Steve Wise, netdev, RDMA mailing list,
Stephen Hemminger
In-Reply-To: <d6d1bd56-5def-aa25-4845-e41d2ea5d667@huawei.com>
[-- Attachment #1: Type: text/plain, Size: 2899 bytes --]
On Wed, Feb 27, 2019 at 07:30:34PM +0800, oulijun wrote:
> 在 2019/2/27 14:41, Leon Romanovsky 写道:
> > From: Leon Romanovsky <leonro@mellanox.com>
> >
> > There is a need to distinguish between driver vs. general exposed
> > attributes. The most common use case is to expose some internal
> > garbage under extremely common and sexy name, e.g. pi, ci e.t.c
> >
> > In order to achieve that, we will add "drv_" prefix to all strings
> > which were received through RDMA_NLDEV_ATTR_DRIVER_* attributes.
> >
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>a
> > ---
> > rdma/utils.c | 34 ++++++++++++++++++++++------------
> > 1 file changed, 22 insertions(+), 12 deletions(-)
> >
> > diff --git a/rdma/utils.c b/rdma/utils.c
> > index 6bc14cd5..1f6bf330 100644
> > --- a/rdma/utils.c
> > +++ b/rdma/utils.c
> > @@ -829,27 +829,37 @@ static int print_driver_entry(struct rd *rd, struct nlattr *key_attr,
> > struct nlattr *val_attr,
> > enum rdma_nldev_print_type print_type)
> > {
> > - const char *key_str = mnl_attr_get_str(key_attr);
> > int attr_type = nla_type(val_attr);
> > + int ret = -EINVAL;
> > + char *key_str;
> > +
> > + if (asprintf(&key_str, "drv_%s", mnl_attr_get_str(key_attr)) == -1)
> > + return -ENOMEM;
> >
> > switch (attr_type) {
> > case RDMA_NLDEV_ATTR_DRIVER_STRING:
> > - return print_driver_string(rd, key_str,
> > - mnl_attr_get_str(val_attr));
> > + ret = print_driver_string(rd, key_str,
> > + mnl_attr_get_str(val_attr));
> > + break;
> > case RDMA_NLDEV_ATTR_DRIVER_S32:
> > - return print_driver_s32(rd, key_str,
> > - mnl_attr_get_u32(val_attr), print_type);
> > + ret = print_driver_s32(rd, key_str, mnl_attr_get_u32(val_attr),
> > + print_type);
> > + break;
> > case RDMA_NLDEV_ATTR_DRIVER_U32:
> > - return print_driver_u32(rd, key_str,
> > - mnl_attr_get_u32(val_attr), print_type);
> > + ret = print_driver_u32(rd, key_str, mnl_attr_get_u32(val_attr),
> > + print_type);
> > + break;
> > case RDMA_NLDEV_ATTR_DRIVER_S64:
> > - return print_driver_s64(rd, key_str,
> > - mnl_attr_get_u64(val_attr), print_type);
> > + ret = print_driver_s64(rd, key_str, mnl_attr_get_u64(val_attr),
> > + print_type);
> > + break;
> > case RDMA_NLDEV_ATTR_DRIVER_U64:
> > - return print_driver_u64(rd, key_str,
> > - mnl_attr_get_u64(val_attr), print_type);
> > + ret = print_driver_u64(rd, key_str, mnl_attr_get_u64(val_attr),
> > + print_type);
> > + break;
> > }
> > - return -EINVAL;
> > + free(key_str);
> > + return ret;
> > }
> >
> > void print_driver_table(struct rd *rd, struct nlattr *tb)
> > --
> > 2.19.1
> >
> >
> > .
> >
> This is fine to me and the implementation is more simple.
>
> Tested-by: Lijun Ou <oulijun@huawei.com>
Thanks a lot.
>
> thanks.
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero
From: Tonghao Zhang @ 2019-02-27 11:46 UTC (permalink / raw)
To: Roi Dayan; +Cc: Saeed Mahameed, gerlitz.or@gmail.com, netdev@vger.kernel.org
In-Reply-To: <bbaaf93a-b9a3-289a-afc3-fcaa5068f322@mellanox.com>
On Tue, Feb 26, 2019 at 9:54 PM Roi Dayan <roid@mellanox.com> wrote:
>
>
>
> On 25/02/2019 12:40, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > When max modify header action is zero, we return -EOPNOTSUPP
> > directly. In this way, we can ignore wrong message info (e.g.
> > "mlx5: parsed 0 pedit actions, can't do more").
> >
> > This happens when offloading pedit actions on mlx VFs.
> >
> > For example:
> > $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
> > flower skip_sw dst_mac 00:10:56:fb:64:e8 \
> > dst_ip 1.1.1.100 src_ip 1.1.1.200 \
> > action pedit ex munge eth src set 00:10:56:b4:5d:20
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> > ---
> > drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > index b38986e..708f819 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
> > @@ -2002,7 +2002,8 @@ static int offload_pedit_fields(struct pedit_headers_action *hdrs,
> > static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> > struct pedit_headers_action *hdrs,
> > int namespace,
> > - struct mlx5e_tc_flow_parse_attr *parse_attr)
> > + struct mlx5e_tc_flow_parse_attr *parse_attr,
> > + struct netlink_ext_ack *extack)
> > {
> > int nkeys, action_size, max_actions;
> >
> > @@ -2015,6 +2016,12 @@ static int alloc_mod_hdr_actions(struct mlx5e_priv *priv,
> > else /* namespace is MLX5_FLOW_NAMESPACE_KERNEL - NIC offloading */
> > max_actions = MLX5_CAP_FLOWTABLE_NIC_RX(priv->mdev, max_modify_header_actions);
> >
> > + if (!max_actions) {
> > + NL_SET_ERR_MSG_MOD(extack,
> > + "don't support pedit actions, can't offload");
>
> can we rephrase that to match the msg style you did in patch 5 ?
Yes, good idea
> i.e. The pedit offload action is not supported
>
>
> > + return -EOPNOTSUPP;
> > + }
> > +
> > /* can get up to crazingly 16 HW actions in 32 bits pedit SW key */
> > max_actions = min(max_actions, nkeys * 16);
> >
> > @@ -2072,7 +2079,8 @@ static int alloc_tc_pedit_action(struct mlx5e_priv *priv, int namespace,
> > u8 cmd;
> >
> > if (!parse_attr->mod_hdr_actions) {
> > - err = alloc_mod_hdr_actions(priv, hdrs, namespace, parse_attr);
> > + err = alloc_mod_hdr_actions(priv, hdrs,
> > + namespace, parse_attr, extack);
> > if (err)
> > goto out_err;
> > }
> >
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net/mlx5e: Return -EOPNOTSUPP when modify header action zero
From: Tonghao Zhang @ 2019-02-27 11:47 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Saeed Mahameed, Linux Netdev List
In-Reply-To: <CAJ3xEMh7GU8W=kfnM3eU4-0CxGyqB_1TvMWcr8WB3AK=xRM6gA@mail.gmail.com>
On Wed, Feb 27, 2019 at 6:37 AM Or Gerlitz <gerlitz.or@gmail.com> wrote:
>
> On Mon, Feb 25, 2019 at 1:06 PM <xiangxia.m.yue@gmail.com> wrote:
>>
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> When max modify header action is zero, we return -EOPNOTSUPP
>> directly. In this way, we can ignore wrong message info (e.g.
>> "mlx5: parsed 0 pedit actions, can't do more").
>>
>> This happens when offloading pedit actions on mlx VFs.
>>
>> For example:
>> $ tc filter add dev mlx5_vf parent ffff: protocol ip prio 1 \
>> flower skip_sw dst_mac 00:10:56:fb:64:e8 \
>> dst_ip 1.1.1.100 src_ip 1.1.1.200 \
>> action pedit ex munge eth src set 00:10:56:b4:5d:20
>
>
>
> this command should work, we support header re-write (pedit offload) for tc NIC rules
>
> Is this CX5 VF? if yes, something broke
No, this is cx4
>
^ permalink raw reply
* Request for suggestion on net device re-naming/re-ordering based on DT alias
From: Harini Katakam @ 2019-02-27 11:54 UTC (permalink / raw)
To: netdev, linux-kernel, Nicolas Ferre, David Miller
Cc: Michal Simek, Harini Katakam, Harini Katakam
Hi,
We've had some users requesting control over net device name order
when multiple ethernet devices are present on a system. I've tried a
few solutions to this and looked it up on forums. But I apologize if
I have missed something.
I know that the current system allocates eth<n> as per probe order
but that is obviously not stably controlled by user (tried DT
re-ordering and defer probe). One solution is to use DT alias names
to write to (net_device)->name as follows:
Devicetree:
aliases {
ethernet0 = &mac1;
ethernet1 = &mac0;
};
Driver probe:
+ /* Read ethernet DT alias id and assign to right device name*/
+ id = of_alias_get_id(np, "ethernet");
+ if (id < 0) {
+ dev_warn(&pdev->dev, "failed to get alias id (%u)\n", id);
+ return id;
+ }
+ snprintf(dev->name, sizeof(dev->name), "eth%d", id);
+
These three drivers seem to have something similar for mdio/phy bus IDs:
drivers/net/ethernet/broadcom/genet/bcmmii.c:409: id =
of_alias_get_id(dn, "eth");
drivers/net/ethernet/samsung/sxgbe/sxgbe_platform.c:43: plat->bus_id =
of_alias_get_id(np, "ethernet");
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c:404:
plat->bus_id = of_alias_get_id(np, "ethernet");
Drawback: This approach will break if alias is not provided for one
of the interfaces on board. Not to mention, there could be systems
with multiple ethernet makes (for ex. Cadence macb and Xilinx axienet)
If one of the drivers does not have an alias read mechanism, it is
possible to have clashing ID assignments. Is there any way this
solution can be changed to be stable/acceptable?
One other alternative I've tried is netdev kernel bootargs but this
device name was not being picked by the kernel:
https://www.kernel.org/doc/html/v4.19/admin-guide/kernel-parameters.html
netdev= <mac1’s interrupt id>, <mac1’s base address>, eth0
netdev=<mac0’s interrupt id>, <mac0’s base address>, eth1
Could you please suggest any alternatives?
Thanks!
Regards,
Harini
^ permalink raw reply
* [PATCH v2 net-next 0/6] net: aquantia: minor bug fixes after static analysis
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh
This patchset fixes minor errors and warnings found by smatch and kasan.
Extra patch is to replace AQ_HW_WAIT_FOR with readx_poll_timeout
to improve readability.
V2:
use readx_poll
resubmitted to net-next since the changeset became quite big.
Igor Russkikh (1):
net: aquantia: fixed instack structure overflow
Nikita Danilov (5):
net: aquantia: fixed memcpy size
net: aquantia: added newline at end of file
net: aquantia: fixed buffer overflow
net: aquantia: replace AQ_HW_WAIT_FOR with readx_poll_timeout_atomic
net: aquantia: use better wrappers for state registers
.../ethernet/aquantia/atlantic/aq_ethtool.c | 2 +-
.../ethernet/aquantia/atlantic/aq_hw_utils.h | 14 +--
.../net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
.../ethernet/aquantia/atlantic/aq_pci_func.c | 2 +
.../aquantia/atlantic/hw_atl/hw_atl_a0.c | 25 ++--
.../aquantia/atlantic/hw_atl/hw_atl_b0.c | 16 ++-
.../aquantia/atlantic/hw_atl/hw_atl_llh.c | 21 ++++
.../aquantia/atlantic/hw_atl/hw_atl_llh.h | 12 ++
.../atlantic/hw_atl/hw_atl_llh_internal.h | 2 +
.../aquantia/atlantic/hw_atl/hw_atl_utils.c | 116 ++++++++++++------
.../atlantic/hw_atl/hw_atl_utils_fw2x.c | 68 +++++++---
11 files changed, 197 insertions(+), 83 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v2 net-next 1/6] net: aquantia: fixed memcpy size
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
From: Nikita Danilov <nikita.danilov@aquantia.com>
Not careful array dereference caused analysis tools
to think there could be memory overflow.
There was actually no corruption because the array is
two dimensional.
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c: 140
aq_ethtool_get_strings() error:
memcpy() '*aq_ethtool_stat_names' too small (32 vs 704)
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
index 38e87eed76b9..a718d7a1f76c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_ethtool.c
@@ -138,7 +138,7 @@ static void aq_ethtool_get_strings(struct net_device *ndev,
u8 *p = data;
if (stringset == ETH_SS_STATS) {
- memcpy(p, *aq_ethtool_stat_names,
+ memcpy(p, aq_ethtool_stat_names,
sizeof(aq_ethtool_stat_names));
p = p + sizeof(aq_ethtool_stat_names);
for (i = 0; i < cfg->vecs; i++) {
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 3/6] net: aquantia: fixed buffer overflow
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
From: Nikita Danilov <nikita.danilov@aquantia.com>
The overflow is detected by smatch:
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c: 175
aq_pci_func_free_irqs() error: buffer overflow 'self->aq_vec' 8 <= 31
In reality msix_entry_mask always restricts number of iterations.
Adding extra condition to make logic clear and smatch happy.
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
index c8b44cdb91c1..0217ff4669a4 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_pci_func.c
@@ -170,6 +170,8 @@ void aq_pci_func_free_irqs(struct aq_nic_s *self)
for (i = 32U; i--;) {
if (!((1U << i) & self->msix_entry_mask))
continue;
+ if (i >= AQ_CFG_VECS_MAX)
+ continue;
if (pdev->msix_enabled)
irq_set_affinity_hint(pci_irq_vector(pdev, i), NULL);
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 5/6] net: aquantia: replace AQ_HW_WAIT_FOR with readx_poll_timeout_atomic
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
From: Nikita Danilov <nikita.danilov@aquantia.com>
David noticed the original define was hiding 'err' variable
reference. Thats confusing and counterintuitive.
Andrew noted the whole macro could be replaced with standard readx_poll
kernel macro. This makes code more readable.
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
.../ethernet/aquantia/atlantic/aq_hw_utils.h | 14 +--
.../aquantia/atlantic/hw_atl/hw_atl_a0.c | 21 +++-
.../aquantia/atlantic/hw_atl/hw_atl_b0.c | 12 +-
.../aquantia/atlantic/hw_atl/hw_atl_llh.c | 21 ++++
.../aquantia/atlantic/hw_atl/hw_atl_llh.h | 12 ++
.../atlantic/hw_atl/hw_atl_llh_internal.h | 2 +
.../aquantia/atlantic/hw_atl/hw_atl_utils.c | 110 +++++++++++++-----
.../atlantic/hw_atl/hw_atl_utils_fw2x.c | 64 +++++++---
8 files changed, 184 insertions(+), 72 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
index dc88a1221f1d..bc711238ca0c 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_hw_utils.h
@@ -14,6 +14,8 @@
#ifndef AQ_HW_UTILS_H
#define AQ_HW_UTILS_H
+#include <linux/iopoll.h>
+
#include "aq_common.h"
#ifndef HIDWORD
@@ -23,18 +25,6 @@
#define AQ_HW_SLEEP(_US_) mdelay(_US_)
-#define AQ_HW_WAIT_FOR(_B_, _US_, _N_) \
-do { \
- unsigned int AQ_HW_WAIT_FOR_i; \
- for (AQ_HW_WAIT_FOR_i = _N_; (!(_B_)) && (AQ_HW_WAIT_FOR_i);\
- --AQ_HW_WAIT_FOR_i) {\
- udelay(_US_); \
- } \
- if (!AQ_HW_WAIT_FOR_i) {\
- err = -ETIME; \
- } \
-} while (0)
-
#define aq_pr_err(...) pr_err(AQ_CFG_DRV_NAME ": " __VA_ARGS__)
#define aq_pr_trace(...) pr_info(AQ_CFG_DRV_NAME ": " __VA_ARGS__)
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 30fdcb9b11fd..f6f8338153a2 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -85,6 +85,7 @@ const struct aq_hw_caps_s hw_atl_a0_caps_aqc109 = {
static int hw_atl_a0_hw_reset(struct aq_hw_s *self)
{
int err = 0;
+ u32 val;
hw_atl_glb_glb_reg_res_dis_set(self, 1U);
hw_atl_pci_pci_reg_res_dis_set(self, 0U);
@@ -95,7 +96,9 @@ static int hw_atl_a0_hw_reset(struct aq_hw_s *self)
hw_atl_glb_soft_res_set(self, 1);
/* check 10 times by 1ms */
- AQ_HW_WAIT_FOR(hw_atl_glb_soft_res_get(self) == 0, 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_glb_soft_res_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
@@ -103,7 +106,9 @@ static int hw_atl_a0_hw_reset(struct aq_hw_s *self)
hw_atl_itr_res_irq_set(self, 1U);
/* check 10 times by 1ms */
- AQ_HW_WAIT_FOR(hw_atl_itr_res_irq_get(self) == 0, 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_itr_res_irq_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
@@ -181,6 +186,7 @@ static int hw_atl_a0_hw_rss_hash_set(struct aq_hw_s *self,
int err = 0;
unsigned int i = 0U;
unsigned int addr = 0U;
+ u32 val;
for (i = 10, addr = 0U; i--; ++addr) {
u32 key_data = cfg->is_rss ?
@@ -188,8 +194,9 @@ static int hw_atl_a0_hw_rss_hash_set(struct aq_hw_s *self,
hw_atl_rpf_rss_key_wr_data_set(self, key_data);
hw_atl_rpf_rss_key_addr_set(self, addr);
hw_atl_rpf_rss_key_wr_en_set(self, 1U);
- AQ_HW_WAIT_FOR(hw_atl_rpf_rss_key_wr_en_get(self) == 0,
- 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_rpf_rss_key_wr_en_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
}
@@ -209,6 +216,7 @@ static int hw_atl_a0_hw_rss_set(struct aq_hw_s *self,
int err = 0;
u16 bitary[1 + (HW_ATL_A0_RSS_REDIRECTION_MAX *
HW_ATL_A0_RSS_REDIRECTION_BITS / 16U)];
+ u32 val;
memset(bitary, 0, sizeof(bitary));
@@ -222,8 +230,9 @@ static int hw_atl_a0_hw_rss_set(struct aq_hw_s *self,
hw_atl_rpf_rss_redir_tbl_wr_data_set(self, bitary[i]);
hw_atl_rpf_rss_redir_tbl_addr_set(self, i);
hw_atl_rpf_rss_redir_wr_en_set(self, 1U);
- AQ_HW_WAIT_FOR(hw_atl_rpf_rss_redir_wr_en_get(self) == 0,
- 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_rpf_rss_redir_wr_en_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
}
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index c4cdc51350b2..7eb5ea948d61 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -173,6 +173,7 @@ static int hw_atl_b0_hw_rss_hash_set(struct aq_hw_s *self,
int err = 0;
unsigned int i = 0U;
unsigned int addr = 0U;
+ u32 val;
for (i = 10, addr = 0U; i--; ++addr) {
u32 key_data = cfg->is_rss ?
@@ -180,8 +181,9 @@ static int hw_atl_b0_hw_rss_hash_set(struct aq_hw_s *self,
hw_atl_rpf_rss_key_wr_data_set(self, key_data);
hw_atl_rpf_rss_key_addr_set(self, addr);
hw_atl_rpf_rss_key_wr_en_set(self, 1U);
- AQ_HW_WAIT_FOR(hw_atl_rpf_rss_key_wr_en_get(self) == 0,
- 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_rpf_rss_key_wr_en_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
}
@@ -201,6 +203,7 @@ static int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
int err = 0;
u16 bitary[1 + (HW_ATL_B0_RSS_REDIRECTION_MAX *
HW_ATL_B0_RSS_REDIRECTION_BITS / 16U)];
+ u32 val;
memset(bitary, 0, sizeof(bitary));
@@ -214,8 +217,9 @@ static int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
hw_atl_rpf_rss_redir_tbl_wr_data_set(self, bitary[i]);
hw_atl_rpf_rss_redir_tbl_addr_set(self, i);
hw_atl_rpf_rss_redir_wr_en_set(self, 1U);
- AQ_HW_WAIT_FOR(hw_atl_rpf_rss_redir_wr_en_get(self) == 0,
- 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_rpf_rss_redir_wr_en_get,
+ self, val, val == 0,
+ 1000U, 10000U);
if (err < 0)
goto err_exit;
}
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
index 939f77e2e117..f72194c77f5f 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.c
@@ -1585,3 +1585,24 @@ void hw_atl_rpfl3l4_ipv6_dest_addr_set(struct aq_hw_s *aq_hw, u8 location,
HW_ATL_RPF_L3_DSTA_ADR(location + i),
ipv6_dest[i]);
}
+
+u32 hw_atl_sem_ram_get(struct aq_hw_s *self)
+{
+ return hw_atl_reg_glb_cpu_sem_get(self, HW_ATL_FW_SM_RAM);
+}
+
+u32 hw_atl_scrpad_get(struct aq_hw_s *aq_hw, u32 scratch_scp)
+{
+ return aq_hw_read_reg(aq_hw,
+ HW_ATL_GLB_CPU_SCRATCH_SCP_ADR(scratch_scp));
+}
+
+u32 hw_atl_scrpad12_get(struct aq_hw_s *self)
+{
+ return hw_atl_scrpad_get(self, 0xB);
+}
+
+u32 hw_atl_scrpad25_get(struct aq_hw_s *self)
+{
+ return hw_atl_scrpad_get(self, 0x18);
+}
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
index 03c570d115fe..40e6c1e44b5b 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh.h
@@ -752,4 +752,16 @@ void hw_atl_rpfl3l4_ipv6_src_addr_set(struct aq_hw_s *aq_hw, u8 location,
void hw_atl_rpfl3l4_ipv6_dest_addr_set(struct aq_hw_s *aq_hw, u8 location,
u32 *ipv6_dest);
+/* get global microprocessor ram semaphore */
+u32 hw_atl_sem_ram_get(struct aq_hw_s *self);
+
+/* get global microprocessor scratch pad register */
+u32 hw_atl_scrpad_get(struct aq_hw_s *aq_hw, u32 scratch_scp);
+
+/* get global microprocessor scratch pad 12 register */
+u32 hw_atl_scrpad12_get(struct aq_hw_s *self);
+
+/* get global microprocessor scratch pad 25 register */
+u32 hw_atl_scrpad25_get(struct aq_hw_s *self);
+
#endif /* HW_ATL_LLH_H */
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
index 8470d92db812..50b4dee5562d 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_llh_internal.h
@@ -2519,4 +2519,6 @@
/* Default value of bitfield l3_da0[1F:0] */
#define HW_ATL_RPF_L3_DSTA_DEFAULT 0x0
+#define HW_ATL_FW_SM_RAM 0x2U
+
#endif /* HW_ATL_LLH_INTERNAL_H */
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index 9b74a3197d7f..e5df40b00afd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -25,7 +25,9 @@
#define HW_ATL_MIF_ADDR 0x0208U
#define HW_ATL_MIF_VAL 0x020CU
-#define HW_ATL_FW_SM_RAM 0x2U
+#define HW_ATL_RPC_CONTROL_ADR 0x0338U
+#define HW_ATL_RPC_STATE_ADR 0x033CU
+
#define HW_ATL_MPI_FW_VERSION 0x18
#define HW_ATL_MPI_CONTROL_ADR 0x0368U
#define HW_ATL_MPI_STATE_ADR 0x036CU
@@ -53,6 +55,12 @@ static int hw_atl_utils_ver_match(u32 ver_expected, u32 ver_actual);
static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
enum hal_atl_utils_fw_state_e state);
+static u32 hw_atl_utils_get_mpi_mbox_tid(struct aq_hw_s *self);
+static u32 hw_atl_utils_mpi_get_state(struct aq_hw_s *self);
+static u32 hw_atl_utils_mif_cmd_get(struct aq_hw_s *self);
+static u32 hw_atl_utils_mif_addr_get(struct aq_hw_s *self);
+static u32 hw_atl_utils_rpc_state_get(struct aq_hw_s *self);
+
int hw_atl_utils_initfw(struct aq_hw_s *self, const struct aq_fw_ops **fw_ops)
{
int err = 0;
@@ -234,6 +242,7 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
{
int k;
u32 boot_exit_code = 0;
+ u32 val;
for (k = 0; k < 1000; ++k) {
u32 flb_status = aq_hw_read_reg(self,
@@ -260,9 +269,11 @@ int hw_atl_utils_soft_reset(struct aq_hw_s *self)
int err = 0;
hw_atl_utils_mpi_set_state(self, MPI_DEINIT);
- AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR) &
- HW_ATL_MPI_STATE_MSK) == MPI_DEINIT,
- 10, 1000U);
+ err = readx_poll_timeout_atomic(hw_atl_utils_mpi_get_state,
+ self, val,
+ (val & HW_ATL_MPI_STATE_MSK) ==
+ MPI_DEINIT,
+ 10, 10000U);
if (err)
return err;
}
@@ -277,10 +288,11 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
u32 *p, u32 cnt)
{
int err = 0;
+ u32 val;
- AQ_HW_WAIT_FOR(hw_atl_reg_glb_cpu_sem_get(self,
- HW_ATL_FW_SM_RAM) == 1U,
- 1U, 10000U);
+ err = readx_poll_timeout_atomic(hw_atl_sem_ram_get,
+ self, val, val == 1U,
+ 1U, 10000U);
if (err < 0) {
bool is_locked;
@@ -299,13 +311,14 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
aq_hw_write_reg(self, HW_ATL_MIF_CMD, 0x00008000U);
if (IS_CHIP_FEATURE(REVISION_B1))
- AQ_HW_WAIT_FOR(a != aq_hw_read_reg(self,
- HW_ATL_MIF_ADDR),
- 1, 1000U);
+ err = readx_poll_timeout_atomic(hw_atl_utils_mif_addr_get,
+ self, val, val != a,
+ 1U, 1000U);
else
- AQ_HW_WAIT_FOR(!(0x100 & aq_hw_read_reg(self,
- HW_ATL_MIF_CMD)),
- 1, 1000U);
+ err = readx_poll_timeout_atomic(hw_atl_utils_mif_cmd_get,
+ self, val,
+ !(val & 0x100),
+ 1U, 1000U);
*(p++) = aq_hw_read_reg(self, HW_ATL_MIF_VAL);
a += 4;
@@ -320,6 +333,7 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
static int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p,
u32 cnt)
{
+ u32 val;
int err = 0;
bool is_locked;
@@ -337,10 +351,11 @@ static int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p,
(0x80000000 | (0xFFFF & (offset * 4))));
hw_atl_mcp_up_force_intr_set(self, 1);
/* 1000 times by 10us = 10ms */
- AQ_HW_WAIT_FOR((aq_hw_read_reg(self,
- 0x32C) & 0xF0000000) !=
- 0x80000000,
- 10, 1000);
+ err = readx_poll_timeout_atomic(hw_atl_scrpad12_get,
+ self, val,
+ (val & 0xF0000000) ==
+ 0x80000000,
+ 10U, 10000U);
}
} else {
u32 offset = 0;
@@ -351,8 +366,10 @@ static int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p,
aq_hw_write_reg(self, 0x20C, p[offset]);
aq_hw_write_reg(self, 0x200, 0xC000);
- AQ_HW_WAIT_FOR((aq_hw_read_reg(self, 0x200U) &
- 0x100) == 0, 10, 1000);
+ err = readx_poll_timeout_atomic(hw_atl_utils_mif_cmd_get,
+ self, val,
+ (val & 0x100) == 0,
+ 1000U, 10000U);
}
}
@@ -395,15 +412,14 @@ static int hw_atl_utils_init_ucp(struct aq_hw_s *self,
hw_atl_reg_glb_cpu_scratch_scp_set(self, 0x00000000U, 25U);
/* check 10 times by 1ms */
- AQ_HW_WAIT_FOR(0U != (self->mbox_addr =
- aq_hw_read_reg(self, 0x360U)), 1000U, 10U);
+ err = readx_poll_timeout_atomic(hw_atl_scrpad25_get,
+ self, self->mbox_addr,
+ self->mbox_addr != 0U,
+ 1000U, 10000U);
return err;
}
-#define HW_ATL_RPC_CONTROL_ADR 0x0338U
-#define HW_ATL_RPC_STATE_ADR 0x033CU
-
struct aq_hw_atl_utils_fw_rpc_tid_s {
union {
u32 val;
@@ -452,10 +468,10 @@ int hw_atl_utils_fw_rpc_wait(struct aq_hw_s *self,
self->rpc_tid = sw.tid;
- AQ_HW_WAIT_FOR(sw.tid ==
- (fw.val =
- aq_hw_read_reg(self, HW_ATL_RPC_STATE_ADR),
- fw.tid), 1000U, 100U);
+ err = readx_poll_timeout_atomic(hw_atl_utils_rpc_state_get,
+ self, fw.val,
+ sw.tid == fw.tid,
+ 1000U, 100000U);
if (fw.len == 0xFFFFU) {
err = hw_atl_utils_fw_rpc_call(self, sw.len);
@@ -559,10 +575,11 @@ static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
transaction_id = mbox.transaction_id;
- AQ_HW_WAIT_FOR(transaction_id !=
- (hw_atl_utils_mpi_read_mbox(self, &mbox),
- mbox.transaction_id),
- 1000U, 100U);
+ err = readx_poll_timeout_atomic(hw_atl_utils_get_mpi_mbox_tid,
+ self, mbox.transaction_id,
+ transaction_id !=
+ mbox.transaction_id,
+ 1000U, 100000U);
if (err < 0)
goto err_exit;
}
@@ -905,6 +922,35 @@ static int aq_fw1x_set_power(struct aq_hw_s *self, unsigned int power_state,
return err;
}
+static u32 hw_atl_utils_get_mpi_mbox_tid(struct aq_hw_s *self)
+{
+ struct hw_atl_utils_mbox_header mbox;
+
+ hw_atl_utils_mpi_read_mbox(self, &mbox);
+
+ return mbox.transaction_id;
+}
+
+static u32 hw_atl_utils_mpi_get_state(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR);
+}
+
+static u32 hw_atl_utils_mif_cmd_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_MIF_CMD);
+}
+
+static u32 hw_atl_utils_mif_addr_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_MIF_ADDR);
+}
+
+static u32 hw_atl_utils_rpc_state_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_RPC_STATE_ADR);
+}
+
const struct aq_fw_ops aq_fw_1x_ops = {
.init = hw_atl_utils_mpi_create,
.deinit = hw_atl_fw1x_deinit,
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index 7de3220d9cab..c628290d7546 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -20,15 +20,14 @@
#include "hw_atl_utils.h"
#include "hw_atl_llh.h"
-#define HW_ATL_FW2X_MPI_EFUSE_ADDR 0x364
-#define HW_ATL_FW2X_MPI_MBOX_ADDR 0x360
#define HW_ATL_FW2X_MPI_RPC_ADDR 0x334
+#define HW_ATL_FW2X_MPI_MBOX_ADDR 0x360
+#define HW_ATL_FW2X_MPI_EFUSE_ADDR 0x364
#define HW_ATL_FW2X_MPI_CONTROL_ADDR 0x368
#define HW_ATL_FW2X_MPI_CONTROL2_ADDR 0x36C
-
#define HW_ATL_FW2X_MPI_STATE_ADDR 0x370
-#define HW_ATL_FW2X_MPI_STATE2_ADDR 0x374
+#define HW_ATL_FW2X_MPI_STATE2_ADDR 0x374
#define HW_ATL_FW2X_CAP_PAUSE BIT(CAPS_HI_PAUSE)
#define HW_ATL_FW2X_CAP_ASYM_PAUSE BIT(CAPS_HI_ASYMMETRIC_PAUSE)
@@ -72,17 +71,24 @@ static int aq_fw2x_set_link_speed(struct aq_hw_s *self, u32 speed);
static int aq_fw2x_set_state(struct aq_hw_s *self,
enum hal_atl_utils_fw_state_e state);
+static u32 aq_fw2x_mbox_get(struct aq_hw_s *self);
+static u32 aq_fw2x_rpc_get(struct aq_hw_s *self);
+static u32 aq_fw2x_state2_get(struct aq_hw_s *self);
+
static int aq_fw2x_init(struct aq_hw_s *self)
{
int err = 0;
/* check 10 times by 1ms */
- AQ_HW_WAIT_FOR(0U != (self->mbox_addr =
- aq_hw_read_reg(self, HW_ATL_FW2X_MPI_MBOX_ADDR)),
- 1000U, 10U);
- AQ_HW_WAIT_FOR(0U != (self->rpc_addr =
- aq_hw_read_reg(self, HW_ATL_FW2X_MPI_RPC_ADDR)),
- 1000U, 100U);
+ err = readx_poll_timeout_atomic(aq_fw2x_mbox_get,
+ self, self->mbox_addr,
+ self->mbox_addr != 0U,
+ 1000U, 10000U);
+
+ err = readx_poll_timeout_atomic(aq_fw2x_rpc_get,
+ self, self->rpc_addr,
+ self->rpc_addr != 0U,
+ 1000U, 100000U);
return err;
}
@@ -286,16 +292,18 @@ static int aq_fw2x_update_stats(struct aq_hw_s *self)
int err = 0;
u32 mpi_opts = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR);
u32 orig_stats_val = mpi_opts & BIT(CAPS_HI_STATISTICS);
+ u32 stats_val;
/* Toggle statistics bit for FW to update */
mpi_opts = mpi_opts ^ BIT(CAPS_HI_STATISTICS);
aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, mpi_opts);
/* Wait FW to report back */
- AQ_HW_WAIT_FOR(orig_stats_val !=
- (aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
- BIT(CAPS_HI_STATISTICS)),
- 1U, 10000U);
+ err = readx_poll_timeout_atomic(aq_fw2x_state2_get,
+ self, stats_val,
+ orig_stats_val != (stats_val &
+ BIT(CAPS_HI_STATISTICS)),
+ 1U, 10000U);
if (err)
return err;
@@ -309,6 +317,7 @@ static int aq_fw2x_set_sleep_proxy(struct aq_hw_s *self, u8 *mac)
unsigned int rpc_size = 0U;
u32 mpi_opts;
int err = 0;
+ u32 val;
rpc_size = sizeof(rpc->msg_id) + sizeof(*cfg);
@@ -337,8 +346,10 @@ static int aq_fw2x_set_sleep_proxy(struct aq_hw_s *self, u8 *mac)
mpi_opts |= HW_ATL_FW2X_CTRL_SLEEP_PROXY;
aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, mpi_opts);
- AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
- HW_ATL_FW2X_CTRL_SLEEP_PROXY), 1U, 10000U);
+ err = readx_poll_timeout_atomic(aq_fw2x_state2_get,
+ self, val,
+ val & HW_ATL_FW2X_CTRL_SLEEP_PROXY,
+ 1U, 10000U);
err_exit:
return err;
@@ -350,6 +361,7 @@ static int aq_fw2x_set_wol_params(struct aq_hw_s *self, u8 *mac)
struct fw2x_msg_wol *msg = NULL;
u32 mpi_opts;
int err = 0;
+ u32 val;
err = hw_atl_utils_fw_rpc_wait(self, &rpc);
if (err < 0)
@@ -374,8 +386,9 @@ static int aq_fw2x_set_wol_params(struct aq_hw_s *self, u8 *mac)
mpi_opts |= HW_ATL_FW2X_CTRL_WOL;
aq_hw_write_reg(self, HW_ATL_FW2X_MPI_CONTROL2_ADDR, mpi_opts);
- AQ_HW_WAIT_FOR((aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR) &
- HW_ATL_FW2X_CTRL_WOL), 1U, 10000U);
+ err = readx_poll_timeout_atomic(aq_fw2x_state2_get,
+ self, val, val & HW_ATL_FW2X_CTRL_WOL,
+ 1U, 10000U);
err_exit:
return err;
@@ -471,6 +484,21 @@ static u32 aq_fw2x_get_flow_control(struct aq_hw_s *self, u32 *fcmode)
return 0;
}
+static u32 aq_fw2x_mbox_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_MBOX_ADDR);
+}
+
+static u32 aq_fw2x_rpc_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_RPC_ADDR);
+}
+
+static u32 aq_fw2x_state2_get(struct aq_hw_s *self)
+{
+ return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
+}
+
const struct aq_fw_ops aq_fw_2x_ops = {
.init = aq_fw2x_init,
.deinit = aq_fw2x_deinit,
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 2/6] net: aquantia: added newline at end of file
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
From: Nikita Danilov <nikita.danilov@aquantia.com>
drivers/net/ethernet/aquantia/atlantic/aq_nic.c: 991:1:
warning: no newline at end of file
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/aq_nic.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
index 0147c037ca96..ff83667410bd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
+++ b/drivers/net/ethernet/aquantia/atlantic/aq_nic.c
@@ -986,4 +986,4 @@ void aq_nic_shutdown(struct aq_nic_s *self)
err_exit:
rtnl_unlock();
-}
\ No newline at end of file
+}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 4/6] net: aquantia: fixed instack structure overflow
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
This is a real stack undercorruption found by kasan build.
The issue did no harm normally because it only overflowed
2 bytes after `bitary` array which on most architectures
were mapped into `err` local.
Fixes: bab6de8fd180 ("net: ethernet: aquantia: Atlantic A0 and B0 specific functions.")
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c | 4 ++--
drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
index 2469ed4d86b9..30fdcb9b11fd 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_a0.c
@@ -207,8 +207,8 @@ static int hw_atl_a0_hw_rss_set(struct aq_hw_s *self,
u32 i = 0U;
u32 num_rss_queues = max(1U, self->aq_nic_cfg->num_rss_queues);
int err = 0;
- u16 bitary[(HW_ATL_A0_RSS_REDIRECTION_MAX *
- HW_ATL_A0_RSS_REDIRECTION_BITS / 16U)];
+ u16 bitary[1 + (HW_ATL_A0_RSS_REDIRECTION_MAX *
+ HW_ATL_A0_RSS_REDIRECTION_BITS / 16U)];
memset(bitary, 0, sizeof(bitary));
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
index b58ca7cb8e9d..c4cdc51350b2 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_b0.c
@@ -199,8 +199,8 @@ static int hw_atl_b0_hw_rss_set(struct aq_hw_s *self,
u32 i = 0U;
u32 num_rss_queues = max(1U, self->aq_nic_cfg->num_rss_queues);
int err = 0;
- u16 bitary[(HW_ATL_B0_RSS_REDIRECTION_MAX *
- HW_ATL_B0_RSS_REDIRECTION_BITS / 16U)];
+ u16 bitary[1 + (HW_ATL_B0_RSS_REDIRECTION_MAX *
+ HW_ATL_B0_RSS_REDIRECTION_BITS / 16U)];
memset(bitary, 0, sizeof(bitary));
--
2.17.1
^ permalink raw reply related
* [PATCH v2 net-next 6/6] net: aquantia: use better wrappers for state registers
From: Igor Russkikh @ 2019-02-27 12:10 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev@vger.kernel.org, Igor Russkikh, Nikita Danilov
In-Reply-To: <cover.1551269343.git.igor.russkikh@aquantia.com>
From: Nikita Danilov <nikita.danilov@aquantia.com>
Replace some direct registers reads with better
online functions.
Signed-off-by: Nikita Danilov <nikita.danilov@aquantia.com>
Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
---
.../net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c | 6 +++---
.../ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
index e5df40b00afd..eb4b99d56081 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils.c
@@ -298,7 +298,7 @@ int hw_atl_utils_fw_downld_dwords(struct aq_hw_s *self, u32 a,
bool is_locked;
hw_atl_reg_glb_cpu_sem_set(self, 1U, HW_ATL_FW_SM_RAM);
- is_locked = hw_atl_reg_glb_cpu_sem_get(self, HW_ATL_FW_SM_RAM);
+ is_locked = hw_atl_sem_ram_get(self);
if (!is_locked) {
err = -ETIME;
goto err_exit;
@@ -337,7 +337,7 @@ static int hw_atl_utils_fw_upload_dwords(struct aq_hw_s *self, u32 a, u32 *p,
int err = 0;
bool is_locked;
- is_locked = hw_atl_reg_glb_cpu_sem_get(self, HW_ATL_FW_SM_RAM);
+ is_locked = hw_atl_sem_ram_get(self);
if (!is_locked) {
err = -ETIME;
goto err_exit;
@@ -602,7 +602,7 @@ static int hw_atl_utils_mpi_set_state(struct aq_hw_s *self,
int hw_atl_utils_mpi_get_link_status(struct aq_hw_s *self)
{
- u32 cp0x036C = aq_hw_read_reg(self, HW_ATL_MPI_STATE_ADR);
+ u32 cp0x036C = hw_atl_utils_mpi_get_state(self);
u32 link_speed_mask = cp0x036C >> HW_ATL_MPI_SPEED_SHIFT;
struct aq_hw_link_status_s *link_status = &self->aq_link_status;
diff --git a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
index c628290d7546..fe6c5658e016 100644
--- a/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
+++ b/drivers/net/ethernet/aquantia/atlantic/hw_atl/hw_atl_utils_fw2x.c
@@ -438,7 +438,7 @@ static int aq_fw2x_get_eee_rate(struct aq_hw_s *self, u32 *rate,
*supported_rates = fw2x_to_eee_mask(caps_hi);
- mpi_state = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
+ mpi_state = aq_fw2x_state2_get(self);
*rate = fw2x_to_eee_mask(mpi_state);
return err;
@@ -468,7 +468,7 @@ static int aq_fw2x_set_flow_control(struct aq_hw_s *self)
static u32 aq_fw2x_get_flow_control(struct aq_hw_s *self, u32 *fcmode)
{
- u32 mpi_state = aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
+ u32 mpi_state = aq_fw2x_state2_get(self);
if (mpi_state & HW_ATL_FW2X_CAP_PAUSE)
if (mpi_state & HW_ATL_FW2X_CAP_ASYM_PAUSE)
--
2.17.1
^ permalink raw reply related
* [iproute2] tc/pedit: Fix wrong pedit ipv6 structure id
From: Dmytro Linkin @ 2019-02-27 12:10 UTC (permalink / raw)
To: netdev; +Cc: Dmytro Linkin
Tc pedit action with more than two ip6 munge in a row cause infinite
loop.
Example:
$ tc filter add dev eth0 protocol ipv6 parent ffff: \
flower ip_proto sctp \
action pedit ex \
munge ip6 hoplimit set 0x1 \
munge ip6 src set 2001:0db8:0:f101::1 \
munge that cause infinite loop
The example command never returns, instead of failing with parse error
as expected. Pedit ipv6 structure has wrong id, which leads to the
creation linked list with one node in tc/m_pedit.c:get_pedit_kind(),
referring to itself. This node is created if command have two ip6 munge
in a row, and any third ip6 munge will cause infinite loop.
Changing this id from "ipv6" to "ip6" solves the problem.
Fixes: f3e1b2448a95 ("pedit: Introduce ipv6 support")
Signed-off-by: Dmytro Linkin <dmitrolin@mellanox.com>
Reviewed-by: Roi Dayan <roid@mellanox.com>
---
tc/p_ip6.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tc/p_ip6.c b/tc/p_ip6.c
index dbfdca42..7cc7997b 100644
--- a/tc/p_ip6.c
+++ b/tc/p_ip6.c
@@ -84,6 +84,6 @@ done:
}
struct m_pedit_util p_pedit_ip6 = {
- .id = "ipv6",
+ .id = "ip6",
.parse_peopt = parse_ip6,
};
--
2.13.6
^ permalink raw reply related
* [PATCH v4] net: phy: Micrel KSZ8061: link failure after cable connect
From: Rajasingh Thavamani @ 2019-02-27 12:13 UTC (permalink / raw)
Cc: t.rajasingh, Rajasingh Thavamani, Alexander Onnasch, Andrew Lunn,
Florian Fainelli, Heiner Kallweit, David S. Miller, netdev,
linux-kernel
With Micrel KSZ8061 PHY, the link may occasionally not come up after
Ethernet cable connect. The vendor's (Microchip, former Micrel) errata
sheet 80000688A.pdf descripes the problem and possible workarounds in
detail, see below.
The batch implements workaround 1, which permanently fixes the issue.
DESCRIPTION
Link-up may not occur properly when the Ethernet cable is initially
connected. This issue occurs more commonly when the cable is connected
slowly, but it may occur any time a cable is connected. This issue occurs
in the auto-negotiation circuit, and will not occur if auto-negotiation
is disabled (which requires that the two link partners be set to the
same speed and duplex).
END USER IMPLICATIONS
When this issue occurs, link is not established. Subsequent cable
plug/unplaug cycle will not correct the issue.
WORk AROUND
There are four approaches to work around this issue:
1. This issue can be prevented by setting bit 15 in MMD device address 1,
register 2, prior to connecting the cable or prior to setting the
Restart Auto-negotiation bit in register 0h. The MMD registers are
accessed via the indirect access registers Dh and Eh, or via the Micrel
EthUtil utility as shown here:
. if using the EthUtil utility (usually with a Micrel KSZ8061
Evaluation Board), type the following commands:
> address 1
> mmd 1
> iw 2 b61a
. Alternatively, write the following registers to write to the
indirect MMD register:
Write register Dh, data 0001h
Write register Eh, data 0002h
Write register Dh, data 4001h
Write register Eh, data B61Ah
2. The issue can be avoided by disabling auto-negotiation in the KSZ8061,
either by the strapping option, or by clearing bit 12 in register 0h.
Care must be taken to ensure that the KSZ8061 and the link partner
will link with the same speed and duplex. Note that the KSZ8061
defaults to full-duplex when auto-negotiation is off, but other
devices may default to half-duplex in the event of failed
auto-negotiation.
3. The issue can be avoided by connecting the cable prior to powering-up
or resetting the KSZ8061, and leaving it plugged in thereafter.
4. If the above measures are not taken and the problem occurs, link can
be recovered by setting the Restart Auto-Negotiation bit in
register 0h, or by resetting or power cycling the device. Reset may
be either hardware reset or software reset (register 0h, bit 15).
PLAN
This errata will not be corrected in the future revision.
Fixes: 7ab59dc15e2f ("drivers/net/phy/micrel_phy: Add support for new PHYs")
Signed-off-by: Alexander Onnasch <alexander.onnasch@landisgyr.com>
Signed-off-by: Rajasingh Thavamani <T.Rajasingh@landisgyr.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
---
drivers/net/phy/micrel.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index b1f959935f50..b7df0295a3ca 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -344,6 +344,17 @@ static int ksz8041_config_aneg(struct phy_device *phydev)
return genphy_config_aneg(phydev);
}
+static int ksz8061_config_init(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = phy_write_mmd(phydev, MDIO_MMD_PMAPMD, MDIO_DEVID1, 0xB61A);
+ if (ret)
+ return ret;
+
+ return kszphy_config_init(phydev);
+}
+
static int ksz9021_load_values_from_of(struct phy_device *phydev,
const struct device_node *of_node,
u16 reg,
@@ -1040,7 +1051,7 @@ static struct phy_driver ksphy_driver[] = {
.name = "Micrel KSZ8061",
.phy_id_mask = MICREL_PHY_ID_MASK,
.features = PHY_BASIC_FEATURES,
- .config_init = kszphy_config_init,
+ .config_init = ksz8061_config_init,
.ack_interrupt = kszphy_ack_interrupt,
.config_intr = kszphy_config_intr,
.suspend = genphy_suspend,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next 2/8] devlink: add PF and VF port flavours
From: Jiri Pirko @ 2019-02-27 12:16 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, oss-drivers, netdev
In-Reply-To: <20190226182436.23811-3-jakub.kicinski@netronome.com>
Tue, Feb 26, 2019 at 07:24:30PM CET, jakub.kicinski@netronome.com wrote:
>Current port flavours cover simple switches and DSA. Add PF
>and VF flavours to cover "switchdev" SR-IOV NICs.
>
>Example devlink user space output:
>
>$ devlink port
>pci/0000:82:00.0/0: type eth netdev p4p1 flavour physical
>pci/0000:82:00.0/10000: type eth netdev eth0 flavour pcie_pf pf 0
>pci/0000:82:00.0/10001: type eth netdev eth1 flavour pcie_vf pf 0 vf 0
>pci/0000:82:00.0/10002: type eth netdev eth2 flavour pcie_vf pf 0 vf 1
I believe that that this output should be in sync with attr names. So
the names should be:
pci_vf
pci_pf
pf_number
vf_number
Like:
pci/0000:82:00.0/10002: type eth netdev eth2 flavour pci_vf pf_number 0 vf_number 1
But that is comment to the userspace part.
>
>$ devlink -jp port
>{
> "port": {
> "pci/0000:82:00.0/0": {
> "type": "eth",
> "netdev": "p4p1",
> "flavour": "physical"
> },
> "pci/0000:82:00.0/10000": {
> "type": "eth",
> "netdev": "eth0",
> "flavour": "pci_pf",
> "pf": 0,
> },
> "pci/0000:82:00.0/10001": {
> "type": "eth",
> "netdev": "eth1",
> "flavour": "pci_vf",
> "pf": 0,
> "vf": 0
> },
> "pci/0000:82:00.0/10002": {
> "type": "eth",
> "netdev": "eth2",
> "flavour": "pci_vf",
> "pf": 0,
> "vf": 1
> }
> }
>}
>
>Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>---
> include/net/devlink.h | 25 ++++++++++--
> include/uapi/linux/devlink.h | 5 +++
> net/core/devlink.c | 73 +++++++++++++++++++++++++++++++-----
> 3 files changed, 91 insertions(+), 12 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 7f5a0bdca228..b5376ef492f1 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -42,9 +42,19 @@ struct devlink {
> struct devlink_port_attrs {
> bool set;
> enum devlink_port_flavour flavour;
>- u32 port_number; /* same value as "split group" */
>- bool split;
>- u32 split_subport_number;
>+ union { /* port identifiers differ per-flavour */
>+ /* PHYSICAL, CPU, DSA */
>+ struct {
>+ bool split;
>+ u32 split_subport_number;
>+ u32 port_number; /* same value as "split group" */
>+ };
>+ /* PCI_PF, PCI_VF */
>+ struct {
>+ u32 pf_number;
>+ u32 vf_number;
>+ } pci;
>+ };
> };
>
> struct devlink_port {
>@@ -568,6 +578,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> enum devlink_port_flavour flavour,
> u32 port_number, bool split,
> u32 split_subport_number);
>+void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number);
> int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len);
> int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
>@@ -782,6 +795,12 @@ static inline void devlink_port_attrs_set(struct devlink_port *devlink_port,
> {
> }
>
>+static inline void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number)
>+{
>+}
>+
> static inline int
> devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len)
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 5bb4ea67d84f..9ce76d4f640d 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -167,6 +167,8 @@ enum devlink_port_flavour {
> DEVLINK_PORT_FLAVOUR_DSA, /* Distributed switch architecture
> * interconnect port.
> */
>+ DEVLINK_PORT_FLAVOUR_PCI_PF, /* PCI Physical function port */
>+ DEVLINK_PORT_FLAVOUR_PCI_VF, /* PCI Physical function port */
> };
>
> enum devlink_param_cmode {
>@@ -332,6 +334,9 @@ enum devlink_attr {
> DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
> DEVLINK_ATTR_FLASH_UPDATE_COMPONENT, /* string */
>
>+ DEVLINK_ATTR_PORT_PCI_PF_NUMBER, /* u32 */
>+ DEVLINK_ATTR_PORT_PCI_VF_NUMBER, /* u32 */
>+
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index a49dee67e66f..af177284830b 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -516,16 +516,35 @@ static int devlink_nl_port_attrs_put(struct sk_buff *msg,
> return 0;
> if (nla_put_u16(msg, DEVLINK_ATTR_PORT_FLAVOUR, attrs->flavour))
> return -EMSGSIZE;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
>- return -EMSGSIZE;
>- if (!attrs->split)
>+
>+ switch (attrs->flavour) {
>+ case DEVLINK_PORT_FLAVOUR_PHYSICAL:
>+ case DEVLINK_PORT_FLAVOUR_CPU:
>+ case DEVLINK_PORT_FLAVOUR_DSA:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER,
>+ attrs->port_number))
>+ return -EMSGSIZE;
>+
>+ if (attrs->split &&
>+ (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
>+ attrs->port_number) ||
>+ nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
>+ attrs->split_subport_number)))
>+ return -EMSGSIZE;
> return 0;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP, attrs->port_number))
>- return -EMSGSIZE;
>- if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_SUBPORT_NUMBER,
>- attrs->split_subport_number))
>- return -EMSGSIZE;
>- return 0;
>+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_VF_NUMBER,
>+ attrs->pci.vf_number))
>+ return -EMSGSIZE;
>+ /* fall through */
>+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
>+ if (nla_put_u32(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
>+ attrs->pci.pf_number))
>+ return -EMSGSIZE;
>+ return 0;
>+ default:
>+ return -EINVAL;
>+ }
> }
>
> static int devlink_nl_port_fill(struct sk_buff *msg, struct devlink *devlink,
>@@ -5410,6 +5429,9 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> {
> struct devlink_port_attrs *attrs = &devlink_port->attrs;
>
>+ WARN_ON(flavour == DEVLINK_PORT_FLAVOUR_PCI_PF ||
>+ flavour == DEVLINK_PORT_FLAVOUR_PCI_VF);
>+
> attrs->set = true;
> attrs->flavour = flavour;
> attrs->port_number = port_number;
>@@ -5419,6 +5441,32 @@ void devlink_port_attrs_set(struct devlink_port *devlink_port,
> }
> EXPORT_SYMBOL_GPL(devlink_port_attrs_set);
>
>+/**
>+ * devlink_port_attrs_pci_set - Set port attributes for a PCI port
>+ *
>+ * @devlink_port: devlink port
>+ * @flavour: flavour of the port (PF or VF only)
>+ * @pf_number: PCI PF number, in multi-host mapping to hosts depends
>+ * on the platform
>+ * @vf_number: PCI VF number within given PF (ignored for PF itself)
>+ */
>+void devlink_port_attrs_pci_set(struct devlink_port *devlink_port,
>+ enum devlink_port_flavour flavour,
>+ u32 pf_number, u32 vf_number)
Maybe nicer would be to have this static and have 2 helpers:
void devlink_port_attrs_pci_pf_set(struct devlink_port *devlink_port,
u32 pf_number)
void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port,
u32 pf_number, u32 vf_number)
Then you need no warnon. Also you won't have dead arg in driver api
case of pf
Other than this the patch looks good to me.
>+{
>+ struct devlink_port_attrs *attrs = &devlink_port->attrs;
>+
>+ WARN_ON(flavour != DEVLINK_PORT_FLAVOUR_PCI_PF &&
>+ flavour != DEVLINK_PORT_FLAVOUR_PCI_VF);
>+
>+ attrs->set = true;
>+ attrs->flavour = flavour;
>+ attrs->pci.pf_number = pf_number;
>+ attrs->pci.vf_number = vf_number;
>+ devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
>+}
>+EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_set);
>+
> int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> char *name, size_t len)
> {
>@@ -5443,6 +5491,13 @@ int devlink_port_get_phys_port_name(struct devlink_port *devlink_port,
> */
> WARN_ON(1);
> return -EINVAL;
>+ case DEVLINK_PORT_FLAVOUR_PCI_PF:
>+ n = snprintf(name, len, "pf%u", attrs->pci.pf_number);
>+ break;
>+ case DEVLINK_PORT_FLAVOUR_PCI_VF:
>+ n = snprintf(name, len, "pf%uvf%u",
>+ attrs->pf_number, attrs->pci.vf_number);
>+ break;
> }
>
> if (n >= len)
>--
>2.19.2
>
^ permalink raw reply
* Re: [PATCH net] ipvs: get sctphdr by sctphoff in sctp_csum_check
From: Simon Horman @ 2019-02-27 12:21 UTC (permalink / raw)
To: Xin Long
Cc: network dev, netfilter-devel, Marcelo Ricardo Leitner,
Neil Horman, pablo
In-Reply-To: <cd1c829cfd9ee916e57c51f6879947f0f88c1eb1.1551094063.git.lucien.xin@gmail.com>
On Mon, Feb 25, 2019 at 07:27:43PM +0800, Xin Long wrote:
> sctp_csum_check() is called by sctp_s/dnat_handler() where it calls
> skb_make_writable() to ensure sctphdr to be linearized.
>
> So there's no need to get sctphdr by calling skb_header_pointer()
> in sctp_csum_check().
>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Acked-by: Simon Horman <horms@verge.net.au>
Pablo,
please consider applying this to nf-next.
> ---
> net/netfilter/ipvs/ip_vs_proto_sctp.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> index b0cd7d0..0ecf241 100644
> --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c
> +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c
> @@ -183,7 +183,7 @@ static int
> sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> {
> unsigned int sctphoff;
> - struct sctphdr *sh, _sctph;
> + struct sctphdr *sh;
> __le32 cmp, val;
>
> #ifdef CONFIG_IP_VS_IPV6
> @@ -193,10 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp)
> #endif
> sctphoff = ip_hdrlen(skb);
>
> - sh = skb_header_pointer(skb, sctphoff, sizeof(_sctph), &_sctph);
> - if (sh == NULL)
> - return 0;
> -
> + sh = (struct sctphdr *)(skb->data + sctphoff);
> cmp = sh->checksum;
> val = sctp_compute_cksum(skb, sctphoff);
>
> --
> 2.1.0
>
^ 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