* [PATCH net-next] cxgb4: Fixes cxgb4_inet6addr_notifier unregister call
From: Hariprasad Shenai @ 2015-01-21 15:27 UTC (permalink / raw)
To: netdev; +Cc: davem, leedom, anish, nirranjan, praveenm, Hariprasad Shenai
commit b5a02f503caa0837 ("cxgb4 : Update ipv6 address handling api") introduced
a regression where unregister cxgb4_inet6addr_notifier wasn't getting called
during module_exit.
Signed-off-by: Hariprasad Shenai <hariprasad@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1147e1e..12c1a3f 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -6290,7 +6290,7 @@ static int __init cxgb4_init_module(void)
static void __exit cxgb4_cleanup_module(void)
{
#if IS_ENABLED(CONFIG_IPV6)
- if (inet6addr_registered && list_empty(&adapter_list)) {
+ if (inet6addr_registered) {
unregister_inet6addr_notifier(&cxgb4_inet6addr_notifier);
inet6addr_registered = false;
}
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Ahmed S. Darwish @ 2015-01-21 15:36 UTC (permalink / raw)
To: Andri Yngvason
Cc: Wolfgang Grandegger, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121150015.31351.6605@shannon>
On Wed, Jan 21, 2015 at 03:00:15PM +0000, Andri Yngvason wrote:
> Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
> > Hi!
...
> > <-- Unplug the cable -->
> >
> > (000.009106) can0 20000080 [8] 00 00 00 00 00 00 08 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{8}{0}}
> > (000.001872) can0 20000080 [8] 00 00 00 00 00 00 10 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{16}{0}}
> [...]
> > error-counter-tx-rx{{80}{0}}
> > (000.001910) can0 20000080 [8] 00 00 00 00 00 00 58 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{88}{0}}
> > (000.001753) can0 20000084 [8] 00 08 00 00 00 00 60 00 ERRORFRAME
> > controller-problem{tx-error-warning}
> Good.
> > bus-error
> > error-counter-tx-rx{{96}{0}}
Nice.
> > (000.001720) can0 20000080 [8] 00 00 00 00 00 00 68 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{104}{0}}
> > (000.001876) can0 20000080 [8] 00 00 00 00 00 00 70 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{112}{0}}
> > (000.001749) can0 20000080 [8] 00 00 00 00 00 00 78 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{120}{0}}
> > (000.001771) can0 20000084 [8] 00 20 00 00 00 00 80 00 ERRORFRAME
> > controller-problem{tx-error-passive}
> Also good.
> > bus-error
> > error-counter-tx-rx{{128}{0}}
Also nice :-)
> > (000.001868) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{128}{0}}
> > (000.001982) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> > bus-error
> > error-counter-tx-rx{{128}{0}}
> >
> > (( Then a continous flood, exactly similar to the above packet, appears.
> > Unfortunately this flooding is a firmware problem. ))
> >
> > <-- Replug the cable, after a good amount of time -->
> >
> Where are the reverse state transitions?
> >
Hmmm...
[ ... ]
>
> Reverse state transitions are missing from the logs. See comments above.
>
When the device is on the _receiving_ end, and I unplug the CAN cable after
introducing some noise to the level of reaching WARNING or PASSIVE, I
receive a BUS_ERROR event with the rxerr count reset back to 0 or 1. In
that case, the driver correctly transitions back the state to ERROR_ACTIVE
and candump produces something similar to:
(000.000362) can0 2000008C [8] 00 40 40 00 00 00 00 01 ERRORFRAME
controller-problem{}
protocol-violation{{back-to-error-active}{}}
bus-error
error-counter-tx-rx{{0}{1}}
which is, AFAIK, the correct behaviour from the driver side.
Meanwhile, when the device is on the _sending_ end and I re-plug the CAN
cable again. Sometimes I receive events with txerr reset to 0 or 1, and
the driver correctly reverts back to ERROR_ACTIVE in that case. But on
another times like the quoted case above, I don't receive any events
resetting txerr back -- only data packets on the bus.
So, What can the driver do given the above?
Thanks,
Darwish
P.S. just in case, I'll also re-check now if the driver unintentionally
drops any important events resetting the txerr count back after a CAN
cable replug -- preventing the code from returning to ERROR_ACTIVE in
the process.
^ permalink raw reply
* unclear ipv6 redirect message (was Re: [PATCH v3 2/3] mfd: lubbock_io: add lubbock_io board)
From: Joe Perches @ 2015-01-21 16:05 UTC (permalink / raw)
To: Russell King - ARM Linux, netdev
Cc: Robert Jarzmik, Lee Jones, Mark Rutland, devicetree, Samuel Ortiz,
Pawel Moll, Ian Campbell, Dmitry Eremin-Solenikov, linux-kernel,
Haojian Zhuang, Rob Herring, Arnd Bergmann, linux-arm-kernel,
Kumar Gala, Daniel Mack
In-Reply-To: <20150121094453.GO26493@n2100.arm.linux.org.uk>
(adding netdev)
On Wed, 2015-01-21 at 09:44 +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 21, 2015 at 08:46:29AM +0100, Robert Jarzmik wrote:
> > Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> >
> > > What I'd suggest (and always have done) is:
> > >
> > > dev_err(&pdev->dev, "couldn't request main irq%d: %d\n",
> > > irq, ret);
> > I like it, it's even more compact, I'll use it for next patch version.
>
> BTW, this is an example why I have the policy of always ensuring that
> the kernel messages print sufficient diagnostics. Right now, I have
> a problem - since I rebooted my firewall a few nights ago, I now get
> on one of my machines:
>
> rt6_redirect: source isn't a valid nexthop for redirect target
>
> and it spews that for a few minutes every 26 hours or so. No further
> information, and it leaves you wondering "well, what was the invalid
> next hop? What was the source?"
>
> Pretty much the only way to try and find out is to leave a tcpdump or
> wireshark running for 24 hours to try and get a dump - which is not
> that easy if you don't have lots of disk space. So, right now, I have
> no way to diagnose the above.
>
> If it printed that information, then I'd be able to see what the
> addresses were, and I'd probably be able to come up with a tcpdump
> filter which didn't involve logging all IPv6 traffic.
>
> Kernel messages need to be smart. If not, they might as well just be
> "The kernel encountered a problem. Abort, Retry or Fail?"
>
^ permalink raw reply
* Re: unclear ipv6 redirect message (was Re: [PATCH v3 2/3] mfd: lubbock_io: add lubbock_io board)
From: Russell King - ARM Linux @ 2015-01-21 16:11 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Robert Jarzmik, Lee Jones, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz, Pawel Moll,
Ian Campbell, Dmitry Eremin-Solenikov,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Haojian Zhuang, Rob Herring,
Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Kumar Gala, Daniel Mack
In-Reply-To: <1421856321.10574.13.camel-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>
On Wed, Jan 21, 2015 at 08:05:21AM -0800, Joe Perches wrote:
> (adding netdev)
I wasn't actually reporting that as an issue; I was using it as an
example. It's from a very old kernel (2.6.27.21) which I run on one
of my old x86 machines.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Wolfgang Grandegger @ 2015-01-21 16:13 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Andri Yngvason, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121153647.GB17070@linux>
On Wed, 21 Jan 2015 10:36:47 -0500, "Ahmed S. Darwish"
<darwish.07@gmail.com> wrote:
> On Wed, Jan 21, 2015 at 03:00:15PM +0000, Andri Yngvason wrote:
>> Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
>> > Hi!
>
> ...
>
>> > <-- Unplug the cable -->
>> >
>> > (000.009106) can0 20000080 [8] 00 00 00 00 00 00 08 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{8}{0}}
>> > (000.001872) can0 20000080 [8] 00 00 00 00 00 00 10 00
For a bus-errors I would also expcect some more information in the
data[2..3] fields. But these are always zero.
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{16}{0}}
>> [...]
>> > error-counter-tx-rx{{80}{0}}
>> > (000.001910) can0 20000080 [8] 00 00 00 00 00 00 58 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{88}{0}}
>> > (000.001753) can0 20000084 [8] 00 08 00 00 00 00 60 00
>> > ERRORFRAME
>> > controller-problem{tx-error-warning}
>> Good.
>> > bus-error
>> > error-counter-tx-rx{{96}{0}}
>
> Nice.
>
>> > (000.001720) can0 20000080 [8] 00 00 00 00 00 00 68 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{104}{0}}
>> > (000.001876) can0 20000080 [8] 00 00 00 00 00 00 70 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{112}{0}}
>> > (000.001749) can0 20000080 [8] 00 00 00 00 00 00 78 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{120}{0}}
>> > (000.001771) can0 20000084 [8] 00 20 00 00 00 00 80 00
>> > ERRORFRAME
>> > controller-problem{tx-error-passive}
>> Also good.
>> > bus-error
>> > error-counter-tx-rx{{128}{0}}
>
> Also nice :-)
>
>> > (000.001868) can0 20000080 [8] 00 00 00 00 00 00 80 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{128}{0}}
>> > (000.001982) can0 20000080 [8] 00 00 00 00 00 00 80 00
>> > ERRORFRAME
>> > bus-error
>> > error-counter-tx-rx{{128}{0}}
>> >
>> > (( Then a continous flood, exactly similar to the above packet,
>> > appears.
>> > Unfortunately this flooding is a firmware problem. ))
>> >
>> > <-- Replug the cable, after a good amount of time -->
>> >
>> Where are the reverse state transitions?
>> >
>
> Hmmm...
>
> [ ... ]
>>
>> Reverse state transitions are missing from the logs. See comments
above.
>>
>
> When the device is on the _receiving_ end, and I unplug the CAN cable
after
> introducing some noise to the level of reaching WARNING or PASSIVE, I
> receive a BUS_ERROR event with the rxerr count reset back to 0 or 1. In
> that case, the driver correctly transitions back the state to
ERROR_ACTIVE
> and candump produces something similar to:
>
> (000.000362) can0 2000008C [8] 00 40 40 00 00 00 00 01
> ERRORFRAME
> controller-problem{}
> protocol-violation{{back-to-error-active}{}}
> bus-error
> error-counter-tx-rx{{0}{1}}
>
> which is, AFAIK, the correct behaviour from the driver side.
>
> Meanwhile, when the device is on the _sending_ end and I re-plug the CAN
> cable again. Sometimes I receive events with txerr reset to 0 or 1, and
> the driver correctly reverts back to ERROR_ACTIVE in that case. But on
> another times like the quoted case above, I don't receive any events
> resetting txerr back -- only data packets on the bus.
Well, the firmware seems to report *only* bus-errors via
CMD_CAN_ERROR_EVENT
messages, also carrying the new state, but no CMD_CHIP_STATE_EVENT just
for
the state changes.
> So, What can the driver do given the above?
Little if the notification does not come.
Wolfgang.
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-21 16:20 UTC (permalink / raw)
To: Ahmed S. Darwish, Olivier Sobrie, Oliver Hartkopp,
Wolfgang Grandegger, Marc Kleine-Budde
Cc: Linux-CAN, netdev, LKML
In-Reply-To: <20150120214537.GB16828@linux>
Quoting Ahmed S. Darwish (2015-01-20 21:45:37)
> From: Ahmed S. Darwish <ahmed.darwish@valeo.com>
>
> Replace most of the can interface's state and error counters
> handling with the new can-dev can_change_state() mechanism.
>
> Suggested-by: Andri Yngvason <andri.yngvason@marel.com>
> Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
> ---
> drivers/net/can/usb/kvaser_usb.c | 114 +++++++++++++++++++--------------------
> 1 file changed, 55 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
> index 971c5f9..0386d3f 100644
> --- a/drivers/net/can/usb/kvaser_usb.c
> +++ b/drivers/net/can/usb/kvaser_usb.c
> @@ -620,40 +620,43 @@ static void kvaser_usb_unlink_tx_urbs(struct kvaser_usb_net_priv *priv)
> }
>
> static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *priv,
> - const struct kvaser_usb_error_summary *es)
> + const struct kvaser_usb_error_summary *es,
> + struct can_frame *cf)
> {
> struct net_device_stats *stats;
> - enum can_state new_state;
> -
> - stats = &priv->netdev->stats;
> - new_state = priv->can.state;
> + enum can_state cur_state, new_state, tx_state, rx_state;
>
> netdev_dbg(priv->netdev, "Error status: 0x%02x\n", es->status);
>
> - if (es->status & M16C_STATE_BUS_OFF) {
> - priv->can.can_stats.bus_off++;
> + stats = &priv->netdev->stats;
> + new_state = cur_state = priv->can.state;
> +
> + if (es->status & M16C_STATE_BUS_OFF)
> new_state = CAN_STATE_BUS_OFF;
> - } else if (es->status & M16C_STATE_BUS_PASSIVE) {
> - if (priv->can.state != CAN_STATE_ERROR_PASSIVE)
> - priv->can.can_stats.error_passive++;
> + else if (es->status & M16C_STATE_BUS_PASSIVE)
> new_state = CAN_STATE_ERROR_PASSIVE;
> - }
>
> if (es->status == M16C_STATE_BUS_ERROR) {
> - if ((priv->can.state < CAN_STATE_ERROR_WARNING) &&
> - ((es->txerr >= 96) || (es->rxerr >= 96))) {
> - priv->can.can_stats.error_warning++;
> + if ((cur_state < CAN_STATE_ERROR_WARNING) &&
> + ((es->txerr >= 96) || (es->rxerr >= 96)))
> new_state = CAN_STATE_ERROR_WARNING;
> - } else if (priv->can.state > CAN_STATE_ERROR_ACTIVE) {
> + else if (cur_state > CAN_STATE_ERROR_ACTIVE)
> new_state = CAN_STATE_ERROR_ACTIVE;
> - }
> }
>
> if (!es->status)
> new_state = CAN_STATE_ERROR_ACTIVE;
>
> + if (new_state != cur_state) {
> + tx_state = (es->txerr >= es->rxerr) ? new_state : 0;
> + rx_state = (es->txerr <= es->rxerr) ? new_state : 0;
> +
> + can_change_state(priv->netdev, cf, tx_state, rx_state);
This (below) is redundant. It doesn't harm but at this point can_change_state
has set priv->can.state to new_state.
> + new_state = priv->can.state;
> + }
> +
> if (priv->can.restart_ms &&
> - (priv->can.state >= CAN_STATE_BUS_OFF) &&
> + (cur_state >= CAN_STATE_BUS_OFF) &&
> (new_state < CAN_STATE_BUS_OFF)) {
> priv->can.can_stats.restarts++;
> }
> @@ -665,18 +668,17 @@ static void kvaser_usb_rx_error_update_can_state(struct kvaser_usb_net_priv *pri
>
> priv->bec.txerr = es->txerr;
> priv->bec.rxerr = es->rxerr;
> - priv->can.state = new_state;
> }
>
> static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> const struct kvaser_msg *msg)
> {
> - struct can_frame *cf;
> + struct can_frame *cf, tmp_cf = { .can_id = CAN_ERR_FLAG, .can_dlc = CAN_ERR_DLC };
> struct sk_buff *skb;
> struct net_device_stats *stats;
> struct kvaser_usb_net_priv *priv;
> struct kvaser_usb_error_summary es = { };
> - enum can_state old_state;
> + enum can_state old_state, new_state;
>
> switch (msg->id) {
> case CMD_CAN_ERROR_EVENT:
> @@ -721,60 +723,54 @@ static void kvaser_usb_rx_error(const struct kvaser_usb *dev,
> }
>
> /* Update all of the can interface's state and error counters before
> - * trying any skb allocation that can actually fail with -ENOMEM.
> + * trying any memory allocation that can actually fail with -ENOMEM.
> + *
> + * We send a temporary stack-allocated error can frame to
> + * can_change_state() for the very same reason.
> + *
> + * TODO: Split can_change_state() responsibility between updating the
> + * can interface's state and counters, and the setting up of can error
> + * frame ID and data to userspace. Remove stack allocation afterwards.
> */
> old_state = priv->can.state;
> - kvaser_usb_rx_error_update_can_state(priv, &es);
> + kvaser_usb_rx_error_update_can_state(priv, &es, &tmp_cf);
> + new_state = priv->can.state;
>
> skb = alloc_can_err_skb(priv->netdev, &cf);
> if (!skb) {
> stats->rx_dropped++;
> return;
> }
> + memcpy(cf, &tmp_cf, sizeof(*cf));
>
> - if (es.status & M16C_STATE_BUS_OFF) {
> - cf->can_id |= CAN_ERR_BUSOFF;
> -
> - if (!priv->can.restart_ms)
> - kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> - netif_carrier_off(priv->netdev);
> - } else if (es.status & M16C_STATE_BUS_PASSIVE) {
> - if (old_state != CAN_STATE_ERROR_PASSIVE) {
> - cf->can_id |= CAN_ERR_CRTL;
> -
> - if (es.txerr || es.rxerr)
> - cf->data[1] = (es.txerr > es.rxerr)
> - ? CAN_ERR_CRTL_TX_PASSIVE
> - : CAN_ERR_CRTL_RX_PASSIVE;
> - else
> - cf->data[1] = CAN_ERR_CRTL_TX_PASSIVE |
> - CAN_ERR_CRTL_RX_PASSIVE;
> + if (new_state != old_state) {
> + if (es.status & M16C_STATE_BUS_OFF) {
> + if (!priv->can.restart_ms)
> + kvaser_usb_simple_msg_async(priv, CMD_STOP_CHIP);
> + netif_carrier_off(priv->netdev);
> + }
> +
This block is wrong. The usage of PROT_ACTIVE is based on a misunderstanding.
It's used in some drivers to signify back-to-error-active but its original
meaning is something completely different, AFAIK.
This is handled in can_change_state() using a new CTRL message; namely:
CAN_ERR_CTRL_ACTIVE. The newest version of can-utils is up to date with this.
> + if (es.status == M16C_STATE_BUS_ERROR) {
> + if ((old_state >= CAN_STATE_ERROR_WARNING) ||
> + (es.txerr < 96 && es.rxerr < 96)) {
> + if (old_state > CAN_STATE_ERROR_ACTIVE) {
> + cf->can_id |= CAN_ERR_PROT;
> + cf->data[2] = CAN_ERR_PROT_ACTIVE;
> + }
> + }
> }
> - }
>
> - if (es.status == M16C_STATE_BUS_ERROR) {
> - if ((old_state < CAN_STATE_ERROR_WARNING) &&
> - ((es.txerr >= 96) || (es.rxerr >= 96))) {
> - cf->can_id |= CAN_ERR_CRTL;
> - cf->data[1] = (es.txerr > es.rxerr)
> - ? CAN_ERR_CRTL_TX_WARNING
> - : CAN_ERR_CRTL_RX_WARNING;
> - } else if (old_state > CAN_STATE_ERROR_ACTIVE) {
This is also redundant, and wrong:
> + if (!es.status) {
> cf->can_id |= CAN_ERR_PROT;
> cf->data[2] = CAN_ERR_PROT_ACTIVE;
> }
> - }
>
> - if (!es.status) {
> - cf->can_id |= CAN_ERR_PROT;
> - cf->data[2] = CAN_ERR_PROT_ACTIVE;
> - }
> -
> - if (priv->can.restart_ms &&
> - (old_state >= CAN_STATE_BUS_OFF) &&
> - (priv->can.state < CAN_STATE_BUS_OFF)) {
> - cf->can_id |= CAN_ERR_RESTARTED;
> - netif_carrier_on(priv->netdev);
> + if (priv->can.restart_ms &&
> + (old_state >= CAN_STATE_BUS_OFF) &&
> + (new_state < CAN_STATE_BUS_OFF)) {
> + cf->can_id |= CAN_ERR_RESTARTED;
> + netif_carrier_on(priv->netdev);
> + }
> }
>
> if (es.error_factor) {
> --
> 1.9.1
Looking over the patch again, I've noticed that there are a few things that are
not quite right.
Marc, could you merge the "move bus_off++" patch before you merge this so that I
won't have to incorporate this patch-set into it?
--
Andri
^ permalink raw reply
* Re: [PATCH] net: dsa: set parent of hwmon device
From: Vivien Didelot @ 2015-01-21 16:26 UTC (permalink / raw)
To: Guenter Roeck
Cc: Florian Fainelli, netdev, David S . Miller, linux-kernel, kernel
In-Reply-To: <CAGVrzcawAF1=RPAJ+0O3B+PA7sDkstTjVjDQL_8C8a8kHL7CoA@mail.gmail.com>
Hi Guenter, Florian,
>>> - ds->hwmon_dev = hwmon_device_register_with_groups(NULL,
>>> + ds->hwmon_dev = hwmon_device_register_with_groups(parent,
>>> ds->hwmon_name, ds, dsa_hwmon_groups);
>>> if (IS_ERR(ds->hwmon_dev))
>>> ds->hwmon_dev = NULL;
>>
>> Looking into my old e-mail, turns out we did not add the parent
>> device because it affected the output of the "sensors" command, and
>> we wanted the device to be handled as 'virtual device' (which implies
>> no parent). That was an explicit part of the patch set (v2 of 'net:
>> dsa: Add support for reporting switch chip temperatures'), compared
>> to v1, which did set the parent device.
I don't know about the "sensors" output, but I found that convenient
from the sysfs side to have a hierarchy of sub-devices logically exposed
at the same place, i.e.:
# ls /sys/devices/platform/dsa.0
driver hwmon net ...
>> I would suggest to keep the code as is.
> Maybe follow-up with a comment adding that above the call to
> hwmon_device_register_with_groups()? I suspect the intent is clear if
> you are deep into hwmon devices, but not necessarily for the reader ;)
Thanks,
-v
^ permalink raw reply
* Re: [PATCH v5 2/5] can: kvaser_usb: Consolidate and unify state change handling
From: Andri Yngvason @ 2015-01-21 16:37 UTC (permalink / raw)
To: Ahmed S. Darwish
Cc: Wolfgang Grandegger, Olivier Sobrie, Oliver Hartkopp,
Marc Kleine-Budde, Linux-CAN, netdev, LKML
In-Reply-To: <20150121153647.GB17070@linux>
Quoting Ahmed S. Darwish (2015-01-21 15:36:47)
> On Wed, Jan 21, 2015 at 03:00:15PM +0000, Andri Yngvason wrote:
> > Quoting Ahmed S. Darwish (2015-01-21 14:43:23)
> > > Hi!
>
> ...
>
> > > <-- Unplug the cable -->
> > >
> > > (000.009106) can0 20000080 [8] 00 00 00 00 00 00 08 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{8}{0}}
> > > (000.001872) can0 20000080 [8] 00 00 00 00 00 00 10 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{16}{0}}
> > [...]
> > > error-counter-tx-rx{{80}{0}}
> > > (000.001910) can0 20000080 [8] 00 00 00 00 00 00 58 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{88}{0}}
> > > (000.001753) can0 20000084 [8] 00 08 00 00 00 00 60 00 ERRORFRAME
> > > controller-problem{tx-error-warning}
> > Good.
> > > bus-error
> > > error-counter-tx-rx{{96}{0}}
>
> Nice.
>
> > > (000.001720) can0 20000080 [8] 00 00 00 00 00 00 68 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{104}{0}}
> > > (000.001876) can0 20000080 [8] 00 00 00 00 00 00 70 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{112}{0}}
> > > (000.001749) can0 20000080 [8] 00 00 00 00 00 00 78 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{120}{0}}
> > > (000.001771) can0 20000084 [8] 00 20 00 00 00 00 80 00 ERRORFRAME
> > > controller-problem{tx-error-passive}
> > Also good.
> > > bus-error
> > > error-counter-tx-rx{{128}{0}}
>
> Also nice :-)
>
> > > (000.001868) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{128}{0}}
> > > (000.001982) can0 20000080 [8] 00 00 00 00 00 00 80 00 ERRORFRAME
> > > bus-error
> > > error-counter-tx-rx{{128}{0}}
> > >
> > > (( Then a continous flood, exactly similar to the above packet, appears.
> > > Unfortunately this flooding is a firmware problem. ))
> > >
> > > <-- Replug the cable, after a good amount of time -->
> > >
> > Where are the reverse state transitions?
> > >
>
> Hmmm...
>
> [ ... ]
> >
> > Reverse state transitions are missing from the logs. See comments above.
> >
>
> When the device is on the _receiving_ end, and I unplug the CAN cable after
> introducing some noise to the level of reaching WARNING or PASSIVE, I
> receive a BUS_ERROR event with the rxerr count reset back to 0 or 1. In
> that case, the driver correctly transitions back the state to ERROR_ACTIVE
> and candump produces something similar to:
>
> (000.000362) can0 2000008C [8] 00 40 40 00 00 00 00 01 ERRORFRAME
> controller-problem{}
> protocol-violation{{back-to-error-active}{}}
> bus-error
> error-counter-tx-rx{{0}{1}}
>
> which is, AFAIK, the correct behaviour from the driver side.
>
> Meanwhile, when the device is on the _sending_ end and I re-plug the CAN
> cable again. Sometimes I receive events with txerr reset to 0 or 1, and
> the driver correctly reverts back to ERROR_ACTIVE in that case. But on
> another times like the quoted case above, I don't receive any events
> resetting txerr back -- only data packets on the bus.
>
> So, What can the driver do given the above?
>
So what you're telling us is that the state does not got back to error-active
unless there is something else transmitting on the bus?
If that's the case, it's almost definitely because state changes aren't
triggering interrupts.
An rx event will give you an interrupt which yields a napi poll which means that
the state will be polled, so in that case you don't need the interrupts.
Look for "Consolidate and unify state change handling" on gmane. There was a lot
of discussion about this kind of issues, especially regarding FlexCAN.
Best regards,
Andri
^ permalink raw reply
* Re: unclear ipv6 redirect message (was Re: [PATCH v3 2/3] mfd: lubbock_io: add lubbock_io board)
From: Joe Perches @ 2015-01-21 16:40 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: netdev, Robert Jarzmik, Lee Jones, Mark Rutland, devicetree,
Samuel Ortiz, Pawel Moll, Ian Campbell, Dmitry Eremin-Solenikov,
linux-kernel, Haojian Zhuang, Rob Herring, Arnd Bergmann,
linux-arm-kernel, Kumar Gala, Daniel Mack
In-Reply-To: <20150121161059.GT26493@n2100.arm.linux.org.uk>
On Wed, 2015-01-21 at 16:11 +0000, Russell King - ARM Linux wrote:
> On Wed, Jan 21, 2015 at 08:05:21AM -0800, Joe Perches wrote:
> > (adding netdev)
>
> I wasn't actually reporting that as an issue; I was using it as an
> example. It's from a very old kernel (2.6.27.21) which I run on one
> of my old x86 machines.
It's still the same code.
If the message can be improved, why not do it?
^ permalink raw reply
* [PATCH 3/3][v2] mdio-mux-gpio: use new gpiod_get_array and gpiod_put_array functions
From: Rojhalat Ibrahim @ 2015-01-21 16:46 UTC (permalink / raw)
To: linux-gpio@vger.kernel.org
Cc: Alexandre Courbot, Linus Walleij, David Miller, netdev
Use the new gpiod_get_array and gpiod_put_array functions for obtaining and
disposing of GPIO descriptors.
Cc: David Miller <davem@davemloft.net>
Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
---
This patch depends on my previous patch "gpiolib: add gpiod_get_array and
gpiod_put_array functions".
v2: use the new interface
Only compile-tested.
drivers/net/phy/mdio-mux-gpio.c | 61 +++++++++++-----------------------------
1 file changed, 18 insertions(+), 43 deletions(-)
diff --git a/drivers/net/phy/mdio-mux-gpio.c b/drivers/net/phy/mdio-mux-gpio.c
index 320eb15..3f6929c 100644
--- a/drivers/net/phy/mdio-mux-gpio.c
+++ b/drivers/net/phy/mdio-mux-gpio.c
@@ -12,33 +12,31 @@
#include <linux/module.h>
#include <linux/phy.h>
#include <linux/mdio-mux.h>
-#include <linux/of_gpio.h>
+#include <linux/gpio/consumer.h>
#define DRV_VERSION "1.1"
#define DRV_DESCRIPTION "GPIO controlled MDIO bus multiplexer driver"
-#define MDIO_MUX_GPIO_MAX_BITS 8
-
struct mdio_mux_gpio_state {
- struct gpio_desc *gpio[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int num_gpios;
+ struct gpio_descs *gpios;
void *mux_handle;
};
static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
void *data)
{
- int values[MDIO_MUX_GPIO_MAX_BITS];
- unsigned int n;
struct mdio_mux_gpio_state *s = data;
+ int values[s->gpios->array_size];
+ unsigned int n;
if (current_child == desired_child)
return 0;
- for (n = 0; n < s->num_gpios; n++) {
+ for (n = 0; n < s->gpios->array_size; n++)
values[n] = (desired_child >> n) & 1;
- }
- gpiod_set_array_cansleep(s->num_gpios, s->gpio, values);
+
+ gpiod_set_array_cansleep(s->gpios->array_size, s->gpios->desc_array,
+ values);
return 0;
}
@@ -46,56 +44,33 @@ static int mdio_mux_gpio_switch_fn(int current_child, int desired_child,
static int mdio_mux_gpio_probe(struct platform_device *pdev)
{
struct mdio_mux_gpio_state *s;
- int num_gpios;
- unsigned int n;
int r;
- if (!pdev->dev.of_node)
- return -ENODEV;
-
- num_gpios = of_gpio_count(pdev->dev.of_node);
- if (num_gpios <= 0 || num_gpios > MDIO_MUX_GPIO_MAX_BITS)
- return -ENODEV;
-
s = devm_kzalloc(&pdev->dev, sizeof(*s), GFP_KERNEL);
if (!s)
return -ENOMEM;
- s->num_gpios = num_gpios;
-
- for (n = 0; n < num_gpios; ) {
- struct gpio_desc *gpio = gpiod_get_index(&pdev->dev, NULL, n,
- GPIOD_OUT_LOW);
- if (IS_ERR(gpio)) {
- r = PTR_ERR(gpio);
- goto err;
- }
- s->gpio[n] = gpio;
- n++;
- }
+ s->gpios = gpiod_get_array(&pdev->dev, NULL, GPIOD_OUT_LOW);
+ if (IS_ERR(s->gpios))
+ return PTR_ERR(s->gpios);
r = mdio_mux_init(&pdev->dev,
mdio_mux_gpio_switch_fn, &s->mux_handle, s);
- if (r == 0) {
- pdev->dev.platform_data = s;
- return 0;
- }
-err:
- while (n) {
- n--;
- gpiod_put(s->gpio[n]);
+ if (r != 0) {
+ gpiod_put_array(s->gpios);
+ return r;
}
- return r;
+
+ pdev->dev.platform_data = s;
+ return 0;
}
static int mdio_mux_gpio_remove(struct platform_device *pdev)
{
- unsigned int n;
struct mdio_mux_gpio_state *s = dev_get_platdata(&pdev->dev);
mdio_mux_uninit(s->mux_handle);
- for (n = 0; n < s->num_gpios; n++)
- gpiod_put(s->gpio[n]);
+ gpiod_put_array(s->gpios);
return 0;
}
--
2.0.5
^ permalink raw reply related
* Re: unclear ipv6 redirect message (was Re: [PATCH v3 2/3] mfd: lubbock_io: add lubbock_io board)
From: Russell King - ARM Linux @ 2015-01-21 16:46 UTC (permalink / raw)
To: Joe Perches
Cc: netdev, Robert Jarzmik, Lee Jones, Mark Rutland, devicetree,
Samuel Ortiz, Pawel Moll, Ian Campbell, Dmitry Eremin-Solenikov,
linux-kernel, Haojian Zhuang, Rob Herring, Arnd Bergmann,
linux-arm-kernel, Kumar Gala, Daniel Mack
In-Reply-To: <1421858444.10574.14.camel@perches.com>
On Wed, Jan 21, 2015 at 08:40:44AM -0800, Joe Perches wrote:
> On Wed, 2015-01-21 at 16:11 +0000, Russell King - ARM Linux wrote:
> > On Wed, Jan 21, 2015 at 08:05:21AM -0800, Joe Perches wrote:
> > > (adding netdev)
> >
> > I wasn't actually reporting that as an issue; I was using it as an
> > example. It's from a very old kernel (2.6.27.21) which I run on one
> > of my old x86 machines.
>
> It's still the same code.
> If the message can be improved, why not do it?
I assume you're taking the responsibility to test anything that comes
out of this then?
I'm not; I tried updating the kernel on the machine to 2.6.32 many
years ago and that was a no-go because of userspace (udev)
incompatibilities.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Tom Herbert @ 2015-01-21 16:51 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CALnjE+rSXpPftiG4Vw3azkN2s2-Ey6ZcZr9Bv422pOdwrvU=tw@mail.gmail.com>
On Wed, Jan 21, 2015 at 1:08 AM, Pravin Shelar <pshelar@nicira.com> wrote:
> On Tue, Jan 20, 2015 at 3:06 PM, Tom Herbert <therbert@google.com> wrote:
>> On Tue, Jan 20, 2015 at 12:25 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>>> Following patch series adds support for Stateless Transport
>>> Tunneling protocol.
>>> STT uses TCP segmentation offload available in most of NIC. On
>>> packet xmit STT driver appends STT header along with TCP header
>>> to the packet. For GSO packet GSO parameters are set according
>>> to tunnel configuration and packet is handed over to networking
>>> stack. This allows use of segmentation offload available in NICs
>>>
>>> Netperf unidirectional test gives ~9.4 Gbits/s performance on 10Gbit
>>> NIC with 1500 byte MTU with two TCP streams.
>>>
>> Having packets marked TCP which really aren't TCP is a rather scary
>> prospect to deploy in a real data center (TCP is kind of an important
>> protocol ;-) ). Can you give some more motivation on this, more data
>> that shows what the benefits are and how this compares to equivalent
>> encapsulation protocols that implement GRO and GSO.
>>
> There are multi-year deployments of STT, So it is already in real data center.
> Biggest advantage is STT does not need new NIC with tunnel offload.
> Any NIC that supports TCP offload can be used to achieve better
> performance.
>
> Following are numbers you asked for.
> Setup: net-next branch on server and client.
> netperf: TCP unidirectional tests with 5 streams. Numbers are averaged
> over 3 runs of 50 sec.
>
Please provides more details on your configuration so that others
might be able to reproduce your results. Also, it would be quite
helpful if you could implement STT as a normal network interface like
VXLAN does so that we can isolate performance of the protocol. For
instance I have no problem getting line rate with VXLAN using 5
streams with or without RCO in my testing. I assume you tested with
OVS and maybe VMs which may have a significant impact beyond the
protocol changes.
Another thing to consider in your analysis is the performance with
flows using small packets. STT should demonstrate better performance
with bulk flows since LSO and LRO are better performing relative to
GSO and GRO. But for flows with small packets, I don't see how there
could be any performance advantage. We already have ways to leverage
simple UDP checksum offload with UDP encapsulations, seems like STT
might just represent unnecessary header overhead in those cases.
> VXLAN:
> CPU
> Client: 1.6
> Server: 14.2
> Throughput: 5.6 Gbit/s
>
> VXLAN with rcsum:
> CPU
> Client: 0.89
> Server: 12.4
> Throughput: 5.8 Gbit/s
>
> STT:
> CPU
> Client: 1.28
> Server: 4.0
> Throughput: 9.5 Gbit/s
>
9.5Gbps? Rounding error or is this 40Gbps or larger than 1500 byte MTU?
Thanks,
Tom
> Thanks,
> Pravin.
^ permalink raw reply
* Re: BW regression after "tcp: refine TSO autosizing"
From: Eric Dumazet @ 2015-01-21 17:01 UTC (permalink / raw)
To: David Laight
Cc: 'Rick Jones', Dave Taht, Eyal Perry, Yuchung Cheng,
Neal Cardwell, Eyal Perry, Or Gerlitz, Linux Netdev List,
Amir Vadai, Yevgeny Petrilin, Saeed Mahameed, Ido Shamay,
Amir Ancel
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D1CAD027E@AcuExch.aculab.com>
On Wed, 2015-01-21 at 12:26 +0000, David Laight wrote:
> From: Of Rick Jones
> > >> Are you saying that at long last, delayed acks as we knew them are
> > >> dead, dead, dead?
> > >
> > > Sorry, I can not parse what you are saying.
> > >
> > > In case you missed it, it has nothing to do with delayed ACK but GRO on
> > > receiver.
> >
> > Dave - assuming I've interpreted Eric's comments correctly, I believe
> > the answer to your question is No. Your desire for a world brimming
> > with ack-every-other purity has not been fulfilled :)
> >
> > However, the engineers formerly at Mentat are probably pleased that a
> > functional near-equivalent to their ACK avoidance heuristic has ended-up
> > being implemented and tacitly accepted, albeit by the back door :)
>
> I must recheck something I discovered a while back with more recent kernels.
> There has been a bad interaction between 'slow start' and 'delayed acks'
> when nagle is disabled on 0 RTT local links with uni-directional traffic.
>
> 'Slow start' would refuse to send more than 4 messages until it received
> an ack (rather than 4 mss of data).
> The receiving system wouldn't send an ack until the timer expired
> (or several mss of data were received) by which time the sender could have
> a lot of data queued.
>
> Due to the 0 RTT and bursty nature of the data 'slow start' happened
> all the time.
Following packetdrill test suggests that current kernel send up to 10
messages without having to wait for any ACK
(IW10)
// Set up production and experiment configs
`../common/defaults.sh`
// Establish a connection.
0.000 socket(..., SOCK_STREAM, IPPROTO_TCP) = 3
0.000 setsockopt(3, SOL_SOCKET, SO_REUSEADDR, [1], 4) = 0
0.000 bind(3, ..., ...) = 0
0.000 listen(3, 1) = 0
0.100 < S 0:0(0) win 32792 <mss 1460,nop,wscale 7>
0.100 > S. 0:0(0) ack 1 <mss 1460,nop,wscale 6>
0.110 < . 1:1(0) ack 1 win 257
0.110 accept(3, ..., ...) = 4
0.200 %{ assert tcpi_snd_cwnd == 10 }%
+0 setsockopt(4, SOL_TCP, TCP_NODELAY, [1], 4) = 0
+0.01 write(4, ..., 100) = 100
+0 > P. 1:101(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 101:201(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 201:301(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 301:401(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 401:501(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 501:601(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 601:701(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 701:801(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 801:901(100) ack 1
+0.01 write(4, ..., 100) = 100
+0 > P. 901:1001(100) ack 1
^ permalink raw reply
* Questions about the sh_eth driver and hardware
From: Ben Hutchings @ 2015-01-21 17:24 UTC (permalink / raw)
To: Nobuhiro Iwamatsu, Mitsuhiro Kimura, Hisashi Nakamura,
Yoshihiro Kaneko
Cc: netdev, ct-linux-kernel
I found several more bugs in the sh_eth driver, but for some of them I'm
really unsure what to do. I only have a manual for the R8A7790 (R-Car
H2) and I don't know how all the other supported chips will behave.
Maybe you can answer some of these questions.
1. When freeing packet buffers, we currently try to stop the DMA engines
by clearing EDRRR and EDTRR but we *don't* wait after that. This seems
unsafe because in general register writes are not serialised with DMA.
The R8A7790 (R-Car H2) manual specifically says that the R bit of EDTRR
(aka CXR2) cannot be cleared by writing to it. I think that we could
stop TX DMA by clearing the active flags of all the descriptors and then
polling the R bit until it clears. What do you think?
As for RX DMA, I think we should wait some time after clearing the R bit
that we can be sure is long enough to transfer one packet. Do you know
how long that could be?
2. In case of a Receive Descriptor Empty error (RDE), we currently read
the RDFAR register to find the next descriptor the DMA engine will use.
But this register is not documented for the R8A7790 and the driver does
not define an offset for it on R-Car chips. The manual doesn't say how
to set the address of the next descriptor to use. Maybe we should
assume that R-Car chips will never skip descriptors after RDE?
Ben.
^ permalink raw reply
* Re: [PATCH 2/2] net: mv643xx_eth: Fix highmem support in non-TSO egress path
From: Russell King - ARM Linux @ 2015-01-21 17:40 UTC (permalink / raw)
To: Ezequiel Garcia; +Cc: netdev, David Miller, B38611, fabio.estevam
In-Reply-To: <1421844850-30886-3-git-send-email-ezequiel.garcia@free-electrons.com>
On Wed, Jan 21, 2015 at 09:54:10AM -0300, Ezequiel Garcia wrote:
> Commit 69ad0dd7af22b61d9e0e68e56b6290121618b0fb
> Author: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> Date: Mon May 19 13:59:59 2014 -0300
>
> net: mv643xx_eth: Use dma_map_single() to map the skb fragments
>
> caused a nasty regression by removing the support for highmem skb
> fragments. By using page_address() to get the address of a fragment's
> page, we are assuming a lowmem page. However, such assumption is incorrect,
> as fragments can be in highmem pages, resulting in very nasty issues.
>
> This commit fixes this by using the skb_frag_dma_map() helper,
> which takes care of mapping the skb fragment properly.
This seems fine, so:
> Fixes: 69ad0dd7af22 ("net: mv643xx_eth: Use dma_map_single() to map the skb fragments")
> Reported-by: Russell King <linux@arm.linux.org.uk>
Reported-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Thanks.
> Signed-off-by: Ezequiel Garcia <ezequiel.garcia@free-electrons.com>
> ---
> drivers/net/ethernet/marvell/mv643xx_eth.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index a62fc38..0c77f0e 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -879,10 +879,8 @@ static void txq_submit_frag_skb(struct tx_queue *txq, struct sk_buff *skb)
> skb_frag_t *this_frag;
> int tx_index;
> struct tx_desc *desc;
> - void *addr;
>
> this_frag = &skb_shinfo(skb)->frags[frag];
> - addr = page_address(this_frag->page.p) + this_frag->page_offset;
> tx_index = txq->tx_curr_desc++;
> if (txq->tx_curr_desc == txq->tx_ring_size)
> txq->tx_curr_desc = 0;
> @@ -902,8 +900,9 @@ static void txq_submit_frag_skb(struct tx_queue *txq, struct sk_buff *skb)
>
> desc->l4i_chk = 0;
> desc->byte_cnt = skb_frag_size(this_frag);
> - desc->buf_ptr = dma_map_single(mp->dev->dev.parent, addr,
> - desc->byte_cnt, DMA_TO_DEVICE);
> + desc->buf_ptr = skb_frag_dma_map(mp->dev->dev.parent,
> + this_frag, 0, desc->byte_cnt,
> + DMA_TO_DEVICE);
> }
> }
>
> @@ -1065,9 +1064,22 @@ static int txq_reclaim(struct tx_queue *txq, int budget, int force)
> reclaimed++;
> txq->tx_desc_count--;
>
> - if (!IS_TSO_HEADER(txq, desc->buf_ptr))
> - dma_unmap_single(mp->dev->dev.parent, desc->buf_ptr,
> - desc->byte_cnt, DMA_TO_DEVICE);
> + if (!IS_TSO_HEADER(txq, desc->buf_ptr)) {
> +
> + /* The first descriptor is either a TSO header or
> + * the linear part of the skb.
> + */
> + if (desc->cmd_sts & TX_FIRST_DESC)
> + dma_unmap_single(mp->dev->dev.parent,
> + desc->buf_ptr,
> + desc->byte_cnt,
> + DMA_TO_DEVICE);
> + else
> + dma_unmap_page(mp->dev->dev.parent,
> + desc->buf_ptr,
> + desc->byte_cnt,
> + DMA_TO_DEVICE);
> + }
>
> if (cmd_sts & TX_ENABLE_INTERRUPT) {
> struct sk_buff *skb = __skb_dequeue(&txq->tx_skb);
> --
> 2.2.1
>
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* [PATCH v2] net: stmmac: add BQL support
From: Beniamino Galvani @ 2015-01-21 18:07 UTC (permalink / raw)
To: David S. Miller
Cc: Giuseppe Cavallaro, Florian Fainelli, Dave Taht, netdev,
linux-kernel, Beniamino Galvani
Add support for Byte Queue Limits to the STMicro MAC driver.
Tested on a Amlogic S802 quad Cortex-A9 board, where the use of BQL
decreases the latency of a high priority ping from ~12ms to ~1ms when
the 100Mbit link is saturated by 20 TCP streams.
Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
---
Changes since v1:
- don't access skb->len after the start of DMA transmission in
stmmac_xmit(), to avoid potential use after free in case tx_lock is
removed in the future
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 118a427..1d74313 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1097,6 +1097,7 @@ static int init_dma_desc_rings(struct net_device *dev, gfp_t flags)
priv->dirty_tx = 0;
priv->cur_tx = 0;
+ netdev_reset_queue(priv->dev);
stmmac_clear_descriptors(priv);
@@ -1300,6 +1301,7 @@ static void stmmac_dma_operation_mode(struct stmmac_priv *priv)
static void stmmac_tx_clean(struct stmmac_priv *priv)
{
unsigned int txsize = priv->dma_tx_size;
+ unsigned int bytes_compl = 0, pkts_compl = 0;
spin_lock(&priv->tx_lock);
@@ -1356,6 +1358,8 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
priv->hw->mode->clean_desc3(priv, p);
if (likely(skb != NULL)) {
+ pkts_compl++;
+ bytes_compl += skb->len;
dev_consume_skb_any(skb);
priv->tx_skbuff[entry] = NULL;
}
@@ -1364,6 +1368,9 @@ static void stmmac_tx_clean(struct stmmac_priv *priv)
priv->dirty_tx++;
}
+
+ netdev_completed_queue(priv->dev, pkts_compl, bytes_compl);
+
if (unlikely(netif_queue_stopped(priv->dev) &&
stmmac_tx_avail(priv) > STMMAC_TX_THRESH(priv))) {
netif_tx_lock(priv->dev);
@@ -1418,6 +1425,7 @@ static void stmmac_tx_err(struct stmmac_priv *priv)
(i == txsize - 1));
priv->dirty_tx = 0;
priv->cur_tx = 0;
+ netdev_reset_queue(priv->dev);
priv->hw->dma->start_tx(priv->ioaddr);
priv->dev->stats.tx_errors++;
@@ -2048,6 +2056,7 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
if (!priv->hwts_tx_en)
skb_tx_timestamp(skb);
+ netdev_sent_queue(dev, skb->len);
priv->hw->dma->enable_dma_transmission(priv->ioaddr);
spin_unlock(&priv->tx_lock);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Pravin Shelar @ 2015-01-21 18:30 UTC (permalink / raw)
To: Tom Herbert; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CA+mtBx9yM+C8GeEHOGTHPVxNB3fJd7LQG0RaC3jywrO9_tQ58A@mail.gmail.com>
On Wed, Jan 21, 2015 at 8:51 AM, Tom Herbert <therbert@google.com> wrote:
> On Wed, Jan 21, 2015 at 1:08 AM, Pravin Shelar <pshelar@nicira.com> wrote:
>> On Tue, Jan 20, 2015 at 3:06 PM, Tom Herbert <therbert@google.com> wrote:
>>> On Tue, Jan 20, 2015 at 12:25 PM, Pravin B Shelar <pshelar@nicira.com> wrote:
>>>> Following patch series adds support for Stateless Transport
>>>> Tunneling protocol.
>>>> STT uses TCP segmentation offload available in most of NIC. On
>>>> packet xmit STT driver appends STT header along with TCP header
>>>> to the packet. For GSO packet GSO parameters are set according
>>>> to tunnel configuration and packet is handed over to networking
>>>> stack. This allows use of segmentation offload available in NICs
>>>>
>>>> Netperf unidirectional test gives ~9.4 Gbits/s performance on 10Gbit
>>>> NIC with 1500 byte MTU with two TCP streams.
>>>>
>>> Having packets marked TCP which really aren't TCP is a rather scary
>>> prospect to deploy in a real data center (TCP is kind of an important
>>> protocol ;-) ). Can you give some more motivation on this, more data
>>> that shows what the benefits are and how this compares to equivalent
>>> encapsulation protocols that implement GRO and GSO.
>>>
>> There are multi-year deployments of STT, So it is already in real data center.
>> Biggest advantage is STT does not need new NIC with tunnel offload.
>> Any NIC that supports TCP offload can be used to achieve better
>> performance.
>>
>> Following are numbers you asked for.
>> Setup: net-next branch on server and client.
>> netperf: TCP unidirectional tests with 5 streams. Numbers are averaged
>> over 3 runs of 50 sec.
>>
> Please provides more details on your configuration so that others
> might be able to reproduce your results. Also, it would be quite
> helpful if you could implement STT as a normal network interface like
> VXLAN does so that we can isolate performance of the protocol. For
> instance I have no problem getting line rate with VXLAN using 5
> streams with or without RCO in my testing. I assume you tested with
> OVS and maybe VMs which may have a significant impact beyond the
> protocol changes.
>
I used bare metal intel servers. All VXLAN tests were done using linux
kernel device without any VMs. All STT tests are done using OVS bridge
and STT port.
> Another thing to consider in your analysis is the performance with
> flows using small packets. STT should demonstrate better performance
> with bulk flows since LSO and LRO are better performing relative to
> GSO and GRO. But for flows with small packets, I don't see how there
> could be any performance advantage. We already have ways to leverage
> simple UDP checksum offload with UDP encapsulations, seems like STT
> might just represent unnecessary header overhead in those cases.
>
All tunneling protocol has performance issue with small packet, I do
not see how is it related to STT patch. STT also make use of checksum
offload, so there should not be much overhead.
>> VXLAN:
>> CPU
>> Client: 1.6
>> Server: 14.2
>> Throughput: 5.6 Gbit/s
>>
>> VXLAN with rcsum:
>> CPU
>> Client: 0.89
>> Server: 12.4
>> Throughput: 5.8 Gbit/s
>>
>> STT:
>> CPU
>> Client: 1.28
>> Server: 4.0
>> Throughput: 9.5 Gbit/s
>>
> 9.5Gbps? Rounding error or is this 40Gbps or larger than 1500 byte MTU?
>
Nope, its same as VXLAN setup, 10Gbps NIC with 1500MTU.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.
From: Pravin Shelar @ 2015-01-21 18:31 UTC (permalink / raw)
To: Joe Stringer; +Cc: netdev, LKML, dev@openvswitch.org
In-Reply-To: <1421778772-11879-6-git-send-email-joestringer@nicira.com>
On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer <joestringer@nicira.com> wrote:
> Previously, flows were manipulated by userspace specifying a full,
> unmasked flow key. This adds significant burden onto flow
> serialization/deserialization, particularly when dumping flows.
>
> This patch adds an alternative way to refer to flows using a
> variable-length "unique flow identifier" (UFID). At flow setup time,
> userspace may specify a UFID for a flow, which is stored with the flow
> and inserted into a separate table for lookup, in addition to the
> standard flow table. Flows created using a UFID must be fetched or
> deleted using the UFID.
>
> All flow dump operations may now be made more terse with OVS_UFID_F_*
> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
> omit the flow key from a datapath operation if the flow has a
> corresponding UFID. This significantly reduces the time spent assembling
> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
> enabled, the datapath only returns the UFID and statistics for each flow
> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
> or more.
>
> Signed-off-by: Joe Stringer <joestringer@nicira.com>
Its almost ready. But I saw minor issues,
few checkpatch.pl failures.
in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
table for legacy case.
^ permalink raw reply
* Re: [PATCH v2] net: stmmac: add BQL support
From: Eric Dumazet @ 2015-01-21 18:37 UTC (permalink / raw)
To: Beniamino Galvani
Cc: David S. Miller, Giuseppe Cavallaro, Florian Fainelli, Dave Taht,
netdev, linux-kernel
In-Reply-To: <1421863647-20048-1-git-send-email-b.galvani@gmail.com>
On Wed, 2015-01-21 at 19:07 +0100, Beniamino Galvani wrote:
> Add support for Byte Queue Limits to the STMicro MAC driver.
>
> Tested on a Amlogic S802 quad Cortex-A9 board, where the use of BQL
> decreases the latency of a high priority ping from ~12ms to ~1ms when
> the 100Mbit link is saturated by 20 TCP streams.
>
> Signed-off-by: Beniamino Galvani <b.galvani@gmail.com>
> ---
> Changes since v1:
> - don't access skb->len after the start of DMA transmission in
> stmmac_xmit(), to avoid potential use after free in case tx_lock is
> removed in the future
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks !
^ permalink raw reply
* Re: [PATCH 11/11] usb: core: fix a race with usb_queue_reset_device()
From: Alan Stern @ 2015-01-21 18:52 UTC (permalink / raw)
To: Olivier Sobrie
Cc: Jan Dumon, Greg Kroah-Hartman,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150121135512.GA9889@hposo>
On Wed, 21 Jan 2015, Olivier Sobrie wrote:
> I tested your patch. It also fixes the problem I observed.
> You can drop mine.
>
> For your info:
>
> My test consists in powering down a usb hso modem while one of its
> serial port is opened. It leads to two URB failures, each urb callback
> queues a reset.
> Without your fix (or without the one I sent), a crash happens after
> less than ~20 power up/down sequences. With your fix, after more than
> 1000 power up/down I don't see any crash.
Okay, I'll submit the patch. Thanks for testing it.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* offline for a bit
From: David Miller @ 2015-01-21 19:11 UTC (permalink / raw)
To: netdev; +Cc: sparclinux
I came down with a very bad stomache virus last night and have been
doing nothing but sleeping and making trips to the bathroom since
yesterday afternoon.
I'm getting better, but it will be a few days before I can catch up
with everything.
Thanks in advance for your patience.
^ permalink raw reply
* Re: [PATCH net-next v13 5/5] openvswitch: Add support for unique flow IDs.
From: Joe Stringer @ 2015-01-21 19:29 UTC (permalink / raw)
To: Pravin Shelar; +Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org, netdev, LKML
In-Reply-To: <CALnjE+qHjf5djBybPQUzoR_+dxDskQJQwtgvSN=G5NW9pckETw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 21 January 2015 at 10:31, Pravin Shelar <pshelar@nicira.com> wrote:
> On Tue, Jan 20, 2015 at 10:32 AM, Joe Stringer <joestringer@nicira.com> wrote:
>> Previously, flows were manipulated by userspace specifying a full,
>> unmasked flow key. This adds significant burden onto flow
>> serialization/deserialization, particularly when dumping flows.
>>
>> This patch adds an alternative way to refer to flows using a
>> variable-length "unique flow identifier" (UFID). At flow setup time,
>> userspace may specify a UFID for a flow, which is stored with the flow
>> and inserted into a separate table for lookup, in addition to the
>> standard flow table. Flows created using a UFID must be fetched or
>> deleted using the UFID.
>>
>> All flow dump operations may now be made more terse with OVS_UFID_F_*
>> flags. For example, the OVS_UFID_F_OMIT_KEY flag allows responses to
>> omit the flow key from a datapath operation if the flow has a
>> corresponding UFID. This significantly reduces the time spent assembling
>> and transacting netlink messages. With all OVS_UFID_F_OMIT_* flags
>> enabled, the datapath only returns the UFID and statistics for each flow
>> during flow dump, increasing ovs-vswitchd revalidator performance by 40%
>> or more.
>>
>> Signed-off-by: Joe Stringer <joestringer@nicira.com>
>
> Its almost ready. But I saw minor issues,
> few checkpatch.pl failures.
> in ovs_flow_cmd_new() we should use unmasked key to lookup in flow
> table for legacy case.
Thanks for review, I can send out a fresh version soon. Should I
resend the whole series or is just a new version of this patch
sufficient?
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Tom Herbert @ 2015-01-21 19:45 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, Linux Netdev List
In-Reply-To: <CALnjE+po5gy7g8ftmiNcnrN_gRL9tzCRHNtn=fxUafFikKB1rg@mail.gmail.com>
> I used bare metal intel servers. All VXLAN tests were done using linux
> kernel device without any VMs. All STT tests are done using OVS bridge
> and STT port.
>
So right off the bat you're running the baseline differently than the
target. Anyway, I cannot replicate your numbers for VXLAN, I see much
better performance and this with pretty old servers and dumb NICs. I
suspect you might not have GSO/GRO properly enabled, but instead of
trying to debug your setup, I'd rather restate my request that you
provide a network interface to STT so we can do our own fair
comparison.
>> Another thing to consider in your analysis is the performance with
>> flows using small packets. STT should demonstrate better performance
>> with bulk flows since LSO and LRO are better performing relative to
>> GSO and GRO. But for flows with small packets, I don't see how there
>> could be any performance advantage. We already have ways to leverage
>> simple UDP checksum offload with UDP encapsulations, seems like STT
>> might just represent unnecessary header overhead in those cases.
>>
> All tunneling protocol has performance issue with small packet, I do
> not see how is it related to STT patch. STT also make use of checksum
> offload, so there should not be much overhead.
>
Given the pervasiveness of these patches and the fact that this is
"modifying" the definition of TCP protocol we see on the wire, I would
like to see more effort into analyzing performance and effects of this
encapsulation. I assume that STT is intended to be used for small
packets so it seems entirely reasonable to ask what exactly the
performance effects are for that case. It's really not that hard to
run some performance numbers with comparisons (TCP_RR, TCP_STREAM,
TCP_CRR, with IPv6, etc.) and report them in the patch set
description. You can look at FOU, GUE, vxlan rco, and the checksum
patches I did for some examples.
>>> VXLAN:
>>> CPU
>>> Client: 1.6
>>> Server: 14.2
>>> Throughput: 5.6 Gbit/s
>>>
>>> VXLAN with rcsum:
>>> CPU
>>> Client: 0.89
>>> Server: 12.4
>>> Throughput: 5.8 Gbit/s
>>>
>>> STT:
>>> CPU
>>> Client: 1.28
>>> Server: 4.0
>>> Throughput: 9.5 Gbit/s
>>>
>> 9.5Gbps? Rounding error or is this 40Gbps or larger than 1500 byte MTU?
>>
> Nope, its same as VXLAN setup, 10Gbps NIC with 1500MTU.
>
That would exceed that theoretical maximum for TCP over 10Gbps
Ethernet. How are you measuring throughput? How many bytes of protocol
headers are in STT case?
Thanks,
Tom
> Thanks.
^ permalink raw reply
* Re: [patch] bridge: simplify br_getlink() a bit
From: Stephen Hemminger @ 2015-01-21 20:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: David S. Miller, bridge, netdev, kernel-janitors
In-Reply-To: <20150121092235.GA19206@mwanda>
On Wed, 21 Jan 2015 12:22:35 +0300
Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Static checkers complain that we should maybe set "ret" before we do the
> "goto out;". They interpret the NULL return from br_port_get_rtnl() as
> a failure and forgetting to set the error code is a common bug in this
> situation.
>
> The code is confusing but it's actually correct. We are returning zero
> deliberately. Let's re-write it a bit to be more clear.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 528cf27..3875ea51 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -311,17 +311,14 @@ errout:
> int br_getlink(struct sk_buff *skb, u32 pid, u32 seq,
> struct net_device *dev, u32 filter_mask)
> {
> - int err = 0;
> struct net_bridge_port *port = br_port_get_rtnl(dev);
>
> if (!port && !(filter_mask & RTEXT_FILTER_BRVLAN) &&
> !(filter_mask & RTEXT_FILTER_BRVLAN_COMPRESSED))
> - goto out;
> + return 0;
>
> - err = br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
> - filter_mask, dev);
> -out:
> - return err;
> + return br_fill_ifinfo(skb, port, pid, seq, RTM_NEWLINK, NLM_F_MULTI,
> + filter_mask, dev);
> }
>
> static int br_vlan_info(struct net_bridge *br, struct net_bridge_port *p,
That looks fine.
Acked-by: Stephen Hemminger <stephen@networkplumber.org>
^ permalink raw reply
* Re: [PATCH net-next 0/3] openvswitch: Add STT support.
From: Eric Dumazet @ 2015-01-21 20:22 UTC (permalink / raw)
To: Tom Herbert; +Cc: Pravin Shelar, David Miller, Linux Netdev List
In-Reply-To: <CA+mtBx-v5dvX-OPGeWJb6uwQGhJ5T6iewD83xJGqXb+Ggf+LQw@mail.gmail.com>
On Wed, 2015-01-21 at 11:45 -0800, Tom Herbert wrote:
> > I used bare metal intel servers. All VXLAN tests were done using linux
> > kernel device without any VMs. All STT tests are done using OVS bridge
> > and STT port.
> >
> So right off the bat you're running the baseline differently than the
> target. Anyway, I cannot replicate your numbers for VXLAN, I see much
> better performance and this with pretty old servers and dumb NICs. I
> suspect you might not have GSO/GRO properly enabled, but instead of
> trying to debug your setup, I'd rather restate my request that you
> provide a network interface to STT so we can do our own fair
> comparison.
I have not read specs nor the patch, but I would expect that with a TCP
stream, you simply can coalesce as many inner segments as you want.
So you could build a single TSO packet containing 400 'messages' of 160
bytes or so.
Something that a datagram tunneling cannot really do, assuming you want
full offloading support.
^ 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