* [PATCH v2] can: c_can: Speed up rx_poll function
From: Markus Pargmann @ 2013-10-29 8:27 UTC (permalink / raw)
To: Marc Kleine-Budde, Wolfgang Grandegger
Cc: Joe Perches, linux-can, netdev, linux-kernel, kernel,
Markus Pargmann
This patch speeds up the rx_poll function by reducing the number of
register reads.
Replace the 32bit register read by a 16bit register read. Currently
the 32bit register read is implemented by using 2 16bit reads. This is
inefficient as we only use the lower 16bit in rx_poll.
The for loop reads the pending interrupts in every iteration. This
leads up to 16 reads of pending interrupts. The patch introduces a new
outer loop to read the pending interrupts as long as 'quota' is above 0.
This reduces the total number of reads.
The third change is to replace the for-loop by a find_next_bit loop.
Tested on AM335x. I removed all 'static' and 'inline' from c_can.c to
see the timings for all functions. I used the function tracer with
trace_stats.
125kbit:
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
With patch:
c_can_do_rx_poll 63939 4268457 us 66.758 us 818790.9 us
1Mbit:
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
With patch:
c_can_do_rx_poll 103034 24220362 us 235.071 us 6016656 us
Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
---
Notes:
Changes in v2:
- Small changes, find_next_bit -> ffs and other
drivers/net/can/c_can/c_can.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
index a668cd4..428681e 100644
--- a/drivers/net/can/c_can/c_can.c
+++ b/drivers/net/can/c_can/c_can.c
@@ -798,17 +798,19 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
u32 num_rx_pkts = 0;
unsigned int msg_obj, msg_ctrl_save;
struct c_can_priv *priv = netdev_priv(dev);
- u32 val = c_can_read_reg32(priv, C_CAN_INTPND1_REG);
+ u16 val;
+
+ /*
+ * It is faster to read only one 16bit register. This is only possible
+ * for a maximum number of 16 objects.
+ */
+ BUILD_BUG_ON_MSG(C_CAN_MSG_OBJ_RX_LAST > 16,
+ "Implementation does not support more message objects than 16");
+
+ while (quota > 0 && (val = priv->read_reg(priv, C_CAN_INTPND1_REG))) {
+ while ((msg_obj = ffs(val)) && quota > 0) {
+ val &= ~BIT(msg_obj - 1);
- for (msg_obj = C_CAN_MSG_OBJ_RX_FIRST;
- msg_obj <= C_CAN_MSG_OBJ_RX_LAST && quota > 0;
- val = c_can_read_reg32(priv, C_CAN_INTPND1_REG),
- msg_obj++) {
- /*
- * as interrupt pending register's bit n-1 corresponds to
- * message object n, we need to handle the same properly.
- */
- if (val & (1 << (msg_obj - 1))) {
c_can_object_get(dev, 0, msg_obj, IF_COMM_ALL &
~IF_COMM_TXRQST);
msg_ctrl_save = priv->read_reg(priv,
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH 4/4] wl1251: spi: add device tree support
From: Kumar Gala @ 2013-10-29 8:28 UTC (permalink / raw)
To: Tomasz Figa
Cc: Mark Rutland, linux-doc, Tony Lindgren, Sebastian Reichel,
Russell King, Sachin Kamat, Stephen Warren, Sebastian Reichel,
Luciano Coelho, devicetree, Pawel Moll, Ian Campbell,
John W. Linville, Rob Herring, Bill Pemberton, linux-omap,
linux-arm-kernel, Greg Kroah-Hartman, linux-wireless,
linux-kernel, Felipe Balbi, Rob Landley, netdev
In-Reply-To: <4893578.SBstXOAcJY@flatron>
On Oct 28, 2013, at 5:38 PM, Tomasz Figa wrote:
> On Monday 28 of October 2013 01:37:34 Kumar Gala wrote:
>> On Oct 27, 2013, at 11:14 AM, Sebastian Reichel wrote:
>>> Add device tree support for the spi variant of wl1251
>>> and document the binding.
>>>
>>> Signed-off-by: Sebastian Reichel <sre@debian.org>
>>> ---
>>> .../devicetree/bindings/net/wireless/ti,wl1251.txt | 36
>>> ++++++++++++++++++++++ drivers/net/wireless/ti/wl1251/spi.c
>>> | 23 ++++++++++---- 2 files changed, 53 insertions(+), 6
>>> deletions(-)
>>> create mode 100644
>>> Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
>>> b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt new
>>> file mode 100644
>>> index 0000000..5f8a154
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
>>> @@ -0,0 +1,36 @@
>>> +* Texas Instruments wl1251 controller
>>> +
>>> +The wl1251 chip can be connected via SPI or via SDIO. The linux
>>> +kernel currently only supports device tree for the SPI variant.
>>> +
>>
>> From the binding I have no idea what this chip actually does, also we
>> don't normally reference linux kernel support in bindings specs (so
>> please remove it).
>>
>> However, what would expect the SDIO binding to look like? Or more
>> specifically, how would you distinguish the SPI vs SDIO
>> binding/connection? I'm wondering if the compatible should be
>> something like "ti,wl1251-spi" and than the sdio can be
>> "ti,wl1251-sdio"
>
> Well, you can easily distinguish an SDIO device from an SPI device by its
> parent node, but...
>
> The binding for SDIO might require different set of properties (other than
> ones inherited from generic SDIO or SPI bindings) than one for SPI. So
> probably different compatible values might be justified.
>
> Did we already have such case before? (maybe some I2C + SPI devices?)
>
>>> +Required properties:
>>> +- compatible : Should be "ti,wl1251"
>>
>> reg is not listed as a required prop.
>
> It is implied by SPI bindings, but it might be a good idea to have this
> stated here as well.
>
>>
>>> +- interrupts : Should contain interrupt line
>>> +- interrupt-parent : Should be the phandle for the interrupt
>>> + controller that services interrupts for this device
>>> +- vio-supply : phandle to regulator providing VIO
>>> +- power-gpio : GPIO connected to chip's PMEN pin
>>
>> should be vendor prefixed: ti,power-gpio
>
> Hmm, out of curiosity, is it a rule for this kind of properties? I can see
> both cases with and without prefixes when grepping for "-gpios" in
> Documentation/devicetree/bindings. We should really have such things
> written down somewhere.
Agreed, it should be part of the various docs we are suppose to produce for review and binding creation guidelines.
>>> +- For additional required properties on SPI, please consult
>>> + Documentation/devicetree/bindings/spi/spi-bus.txt
>>> +
>>> +Optional properties:
>>> +- ti,use-eeprom : If found, configuration will be loaded from eeprom.
>>
>> can you be a bit more specific on what cfg will be loaded. Also, is
>> this property a boolean, if so how do I know which eeprom the cfg is
>> loaded from (is it one that is directly connected to the wl1251?
>
> Maybe one from ti,has-eeprom or ti,config-eeprom would be better name for
> this property?
Probably, ti,wl1251-has-eeprom or something like that would be better. However, I'm not going to get too caught up on names of properties.
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH] bgmac: don't update slot on skb alloc/dma mapping error
From: Rafał Miłecki @ 2013-10-29 8:28 UTC (permalink / raw)
To: Nathan Hintz; +Cc: Network Development
In-Reply-To: <BLU0-SMTP200803D333FF3A2F15AB82DAC090@phx.gbl>
2013/10/29 Nathan Hintz <nlhintz@hotmail.com>:
> On Tue, 29 Oct 2013 07:52:58 +0100
> Rafał Miłecki <zajec5@gmail.com> wrote:
>
>> 2013/10/29 Nathan Hintz <nlhintz@hotmail.com>:
>> > Don't update the slot in "bgmac_dma_rx_skb_for_slot" unless both the
>> > skb alloc and dma mapping are successful; and free the newly allocated
>> > skb if a dma mapping error occurs. This will prevent an skb leak upon
>> > returning when an error occurs.
>>
>> In case of bgmac_dma_rx_skb_for_slot failure we're giving up anyway
>> (and freeing everything), but with your patch code is simpler to
>> understand, so I'm OK with that.
>>
>> Acked-by: Rafał Miłecki <zajec5@gmail.com>
>>
>
> I might be misunderstanding; but it in the case of failure, it appeared to me
> that the currently received packet was dropped and the old skb would continue
> to be assigned to the slot and would be used to receive future packets (this
> would continue until bgmac_dma_rx_skb_for_slot was successful).
I was commenting on current usage (bgmac_dma_alloc), not my WIP patch
for bgmac_dma_rx_read :)
Your patch will be helpful for my bgmac_dma_rx_read rework.
^ permalink raw reply
* Re: [PATCH v2] can: c_can: Speed up rx_poll function
From: Joe Perches @ 2013-10-29 8:31 UTC (permalink / raw)
To: Markus Pargmann
Cc: Marc Kleine-Budde, Wolfgang Grandegger, linux-can, netdev,
linux-kernel, kernel
In-Reply-To: <1383035267-19604-1-git-send-email-mpa@pengutronix.de>
On Tue, 2013-10-29 at 09:27 +0100, Markus Pargmann wrote:
> This patch speeds up the rx_poll function by reducing the number of
> register reads.
[]
> The third change is to replace the for-loop by a find_next_bit loop.
You need to update the commit message.
> diff --git a/drivers/net/can/c_can/c_can.c b/drivers/net/can/c_can/c_can.c
[]
> @@ -798,17 +798,19 @@ static int c_can_do_rx_poll(struct net_device *dev, int quota)
[]
> + while (quota > 0 && (val = priv->read_reg(priv, C_CAN_INTPND1_REG))) {
> + while ((msg_obj = ffs(val)) && quota > 0) {
> + val &= ~BIT(msg_obj - 1);
^ permalink raw reply
* Re: [PATCH] net/cdc_ncm: fix null pointer panic at usbnet_link_change
From: Bjørn Mork @ 2013-10-29 8:41 UTC (permalink / raw)
To: Du, ChangbinX
Cc: oliver@neukum.org, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <0C18FE92A7765D4EB9EE5D38D86A563A019F450F@SHSMSX103.ccr.corp.intel.com>
"Du, ChangbinX" <changbinx.du@intel.com> writes:
> From: "Du, Changbin" <changbinx.du@intel.com>
>
> In cdc_ncm_bind() function, it call cdc_ncm_bind_common() to setup usb.
> But cdc_ncm_bind_common() may meet error and cause usbnet_disconnect()
> be called which calls free_netdev(net).
I am sure you are right, but I really don't see how that can happen.
AFAICS, we ensure that the intfdata is set to NULL before calling
usb_driver_release_interface() in the error cleanup in
cdc_ncm_bind_common():
error2:
usb_set_intfdata(ctx->control, NULL);
usb_set_intfdata(ctx->data, NULL);
if (ctx->data != ctx->control)
usb_driver_release_interface(driver, ctx->data);
error:
cdc_ncm_free((struct cdc_ncm_ctx *)dev->data[0]);
dev->data[0] = 0;
dev_info(&dev->udev->dev, "bind() failure\n");
return -ENODEV;
Thus we hit the test in disconnect, and usbnet_disconnect() is never
called:
static void cdc_ncm_disconnect(struct usb_interface *intf)
{
struct usbnet *dev = usb_get_intfdata(intf);
if (dev == NULL)
return; /* already disconnected */
usbnet_disconnect(intf);
}
So we should really be OK, but we are not???? I would appreciate it if
anyone took the time to feed me why, with a very small spoon...
> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
> index 43afde8..af37ecf 100644
> --- a/drivers/net/usb/cdc_ncm.c
> +++ b/drivers/net/usb/cdc_ncm.c
> @@ -603,14 +603,15 @@ static int cdc_ncm_bind(struct usbnet *dev, struct usb_interface *intf)
>
> /* NCM data altsetting is always 1 */
> ret = cdc_ncm_bind_common(dev, intf, 1);
> -
> - /*
> - * We should get an event when network connection is "connected" or
> - * "disconnected". Set network connection in "disconnected" state
> - * (carrier is OFF) during attach, so the IP network stack does not
> - * start IPv6 negotiation and more.
> - */
> - usbnet_link_change(dev, 0, 0);
> + if (!ret) {
> + /*
> + * We should get an event when network connection is "connected"
> + * or "disconnected". Set network connection in "disconnected"
> + * state (carrier is OFF) during attach, so the IP network stack
> + * does not start IPv6 negotiation and more.
> + */
> + usbnet_link_change(dev, 0, 0);
> + }
> return ret;
> }
This change does of course make sense in any case, so no objections
there. But maybe you could modify cdc_ncm to set the new FLAG_LINK_INTR
instead, and completely drop the usbnet_link_change() from this driver?
This will make usbnet_probe() handle the call, and it will avoid it if
cdc_ncm_bind() failed.
Bjørn
^ permalink raw reply
* Re: [PATCH net] virtio-net: correctly handle cpu hotplug notifier during resuming
From: Wanlong Gao @ 2013-10-29 8:42 UTC (permalink / raw)
To: Jason Wang, rusty, mst, virtualization, netdev, linux-kernel
In-Reply-To: <1383030667-14343-1-git-send-email-jasowang@redhat.com>
On 10/29/2013 03:11 PM, Jason Wang wrote:
> commit 3ab098df35f8b98b6553edc2e40234af512ba877 (virtio-net: don't respond to
> cpu hotplug notifier if we're not ready) tries to bypass the cpu hotplug
> notifier by checking the config_enable and does nothing is it was false. So it
> need to try to hold the config_lock mutex which may happen in atomic
> environment which leads the following warnings:
>
> [ 622.944441] CPU0 attaching NULL sched-domain.
> [ 622.944446] CPU1 attaching NULL sched-domain.
> [ 622.944485] CPU0 attaching NULL sched-domain.
> [ 622.950795] BUG: sleeping function called from invalid context at kernel/mutex.c:616
> [ 622.950796] in_atomic(): 1, irqs_disabled(): 1, pid: 10, name: migration/1
> [ 622.950796] no locks held by migration/1/10.
> [ 622.950798] CPU: 1 PID: 10 Comm: migration/1 Not tainted 3.12.0-rc5-wl-01249-gb91e82d #317
> [ 622.950799] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
> [ 622.950802] 0000000000000000 ffff88001d42dba0 ffffffff81a32f22 ffff88001bfb9c70
> [ 622.950803] ffff88001d42dbb0 ffffffff810edb02 ffff88001d42dc38 ffffffff81a396ed
> [ 622.950805] 0000000000000046 ffff88001d42dbe8 ffffffff810e861d 0000000000000000
> [ 622.950805] Call Trace:
> [ 622.950810] [<ffffffff81a32f22>] dump_stack+0x54/0x74
> [ 622.950815] [<ffffffff810edb02>] __might_sleep+0x112/0x114
> [ 622.950817] [<ffffffff81a396ed>] mutex_lock_nested+0x3c/0x3c6
> [ 622.950818] [<ffffffff810e861d>] ? up+0x39/0x3e
> [ 622.950821] [<ffffffff8153ea7c>] ? acpi_os_signal_semaphore+0x21/0x2d
> [ 622.950824] [<ffffffff81565ed1>] ? acpi_ut_release_mutex+0x5e/0x62
> [ 622.950828] [<ffffffff816d04ec>] virtnet_cpu_callback+0x33/0x87
> [ 622.950830] [<ffffffff81a42576>] notifier_call_chain+0x3c/0x5e
> [ 622.950832] [<ffffffff810e86a8>] __raw_notifier_call_chain+0xe/0x10
> [ 622.950835] [<ffffffff810c5556>] __cpu_notify+0x20/0x37
> [ 622.950836] [<ffffffff810c5580>] cpu_notify+0x13/0x15
> [ 622.950838] [<ffffffff81a237cd>] take_cpu_down+0x27/0x3a
> [ 622.950841] [<ffffffff81136289>] stop_machine_cpu_stop+0x93/0xf1
> [ 622.950842] [<ffffffff81136167>] cpu_stopper_thread+0xa0/0x12f
> [ 622.950844] [<ffffffff811361f6>] ? cpu_stopper_thread+0x12f/0x12f
> [ 622.950847] [<ffffffff81119710>] ? lock_release_holdtime.part.7+0xa3/0xa8
> [ 622.950848] [<ffffffff81135e4b>] ? cpu_stop_should_run+0x3f/0x47
> [ 622.950850] [<ffffffff810ea9b0>] smpboot_thread_fn+0x1c5/0x1e3
> [ 622.950852] [<ffffffff810ea7eb>] ? lg_global_unlock+0x67/0x67
> [ 622.950854] [<ffffffff810e36b7>] kthread+0xd8/0xe0
> [ 622.950857] [<ffffffff81a3bfad>] ? wait_for_common+0x12f/0x164
> [ 622.950859] [<ffffffff810e35df>] ? kthread_create_on_node+0x124/0x124
> [ 622.950861] [<ffffffff81a45ffc>] ret_from_fork+0x7c/0xb0
> [ 622.950862] [<ffffffff810e35df>] ? kthread_create_on_node+0x124/0x124
> [ 622.950876] smpboot: CPU 1 is now offline
> [ 623.194556] SMP alternatives: lockdep: fixing up alternatives
> [ 623.194559] smpboot: Booting Node 0 Processor 1 APIC 0x1
> ...
>
> A correct fix is to unregister the hotcpu notifier during restore and register a
> new one in resume.
>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Tested-by: Fengguang Wu <fengguang.wu@intel.com>
> Cc: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Thank you for your fix.
Reviewed-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
> ---
> This patch is needed for 3.8 and above.
> ---
> drivers/net/virtio_net.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 9fbdfcd..bbc9cb8 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1118,11 +1118,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> {
> struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
>
> - mutex_lock(&vi->config_lock);
> -
> - if (!vi->config_enable)
> - goto done;
> -
> switch(action & ~CPU_TASKS_FROZEN) {
> case CPU_ONLINE:
> case CPU_DOWN_FAILED:
> @@ -1136,8 +1131,6 @@ static int virtnet_cpu_callback(struct notifier_block *nfb,
> break;
> }
>
> -done:
> - mutex_unlock(&vi->config_lock);
> return NOTIFY_OK;
> }
>
> @@ -1699,6 +1692,8 @@ static int virtnet_freeze(struct virtio_device *vdev)
> struct virtnet_info *vi = vdev->priv;
> int i;
>
> + unregister_hotcpu_notifier(&vi->nb);
> +
> /* Prevent config work handler from accessing the device */
> mutex_lock(&vi->config_lock);
> vi->config_enable = false;
> @@ -1747,6 +1742,10 @@ static int virtnet_restore(struct virtio_device *vdev)
> virtnet_set_queues(vi, vi->curr_queue_pairs);
> rtnl_unlock();
>
> + err = register_hotcpu_notifier(&vi->nb);
> + if (err)
> + return err;
> +
> return 0;
> }
> #endif
>
^ permalink raw reply
* Re: [PATCH v2] can: c_can: Speed up rx_poll function
From: Markus Pargmann @ 2013-10-29 8:58 UTC (permalink / raw)
To: Joe Perches
Cc: Marc Kleine-Budde, Wolfgang Grandegger, linux-can, netdev,
linux-kernel, kernel
In-Reply-To: <1383035688.2713.2.camel@joe-AO722>
On Tue, Oct 29, 2013 at 01:34:48AM -0700, Joe Perches wrote:
> On Tue, 2013-10-29 at 09:27 +0100, Markus Pargmann wrote:
> > This patch speeds up the rx_poll function by reducing the number of
> > register reads.
> []
> > 125kbit:
> > Function Hit Time Avg s^2
> > -------- --- ---- --- ---
> > c_can_do_rx_poll 63960 10168178 us 158.977 us 1493056 us
> > With patch:
> > c_can_do_rx_poll 63939 4268457 us 66.758 us 818790.9 us
> >
> > 1Mbit:
> > Function Hit Time Avg s^2
> > -------- --- ---- --- ---
> > c_can_do_rx_poll 69489 30049498 us 432.435 us 9271851 us
> > With patch:
> > c_can_do_rx_poll 103034 24220362 us 235.071 us 6016656 us
>
> Also nicer if you updated the timings table
>
>
Yes I just measured the timings again:
./perf_can_test.sh 125000 30
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_poll 127882 6178806 us 48.316 us 4393411 us
c_can_do_rx_poll 63941 3764057 us 58.867 us 776162.2 us
c_can_enable_all_interrupts 255764 2213697 us 8.655 us 1093934 us
c_can_plat_write_reg_aligned_t 807252 1607115 us 1.990 us 10053684 us
c_can_isr 127882 1220001 us 9.540 us 789.495 us
c_can_object_put 119887 1039222 us 8.668 us 1608.668 us
c_can_plat_read_reg_aligned_to 1015072 1033283 us 1.017 us 7021.465 us
c_can_read_msg_object 63943 1026159 us 16.048 us 718.464 us
c_can_activate_all_lower_rx_ms 7992 755782.4 us 94.567 us 55.270 us
c_can_mark_rx_msg_obj 55951 709072.1 us 12.673 us 39.974 us
c_can_object_get 63943 555669.2 us 8.690 us 96.211 us
c_can_msg_obj_is_busy 183830 527826.1 us 2.871 us 7289.221 us
alloc_can_skb 63943 170966.6 us 2.673 us 165.765 us
c_can_has_and_handle_berr 63941 47063.18 us 0.736 us 2.757 us
./perf_can_test.sh 1000000 30
Function Hit Time Avg s^2
-------- --- ---- --- ---
c_can_poll 270678 30290751 us 111.906 us 5809627 us
c_can_do_rx_poll 207109 24322185 us 117.436 us 171469047 us
c_can_object_put 841431 7278794 us 8.650 us 305841.0 us
c_can_plat_write_reg_aligned_t 4037180 6244636 us 1.546 us 4581066 us
c_can_read_msg_object 453988 6033464 us 13.289 us 128809.6 us
c_can_enable_all_interrupts 541342 5849826 us 10.806 us 22458900 us
c_can_activate_all_lower_rx_ms 55349 5237761 us 94.631 us 19004.79 us
c_can_mark_rx_msg_obj 387429 4865632 us 12.558 us 380606.4 us
c_can_plat_read_reg_aligned_to 4597629 4633247 us 1.007 us 315286.2 us
c_can_object_get 453988 3919692 us 8.633 us 59847.76 us
c_can_msg_obj_is_busy 1295419 3706862 us 2.861 us 316655.7 us
c_can_isr 270671 2496734 us 9.224 us 530.967 us
alloc_can_skb 453988 856917.4 us 1.887 us 18157.68 us
c_can_activate_rx_msg_obj 11210 141177.4 us 12.593 us 123.068 us
c_can_has_and_handle_berr 63569 44995.85 us 0.707 us 12.780 us
It is interesting that the number of hits for c_can_do_rx_poll is twice as much
as it was with find_next_bit. Unfortunately this reduces the overall benefit of
this patch. Any ideas how to increase the number of waiting message objects we
handle in one poll call?
Regards,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: Bug in skb_segment: fskb->len != len
From: Christoph Paasch @ 2013-10-29 9:08 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Herbert Xu, netdev
In-Reply-To: <1383009308.5464.2.camel@edumazet-glaptop.roam.corp.google.com>
On 28/10/13 - 18:15:08, Eric Dumazet wrote:
> On Mon, 2013-10-28 at 06:21 -0700, Eric Dumazet wrote:
>
> > But we also need to fix the skb_segment() bug anyway.
>
> Hi Christoph
>
> I cooked a minimal patch, could you please try it ?
>
> I'll refactor skb_segment() to be smarter for the next release
> (linux-3.14).
>
> Thanks !
>
> net/core/skbuff.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
Ok, my router does not crash anymore with my workload.
Thanks for fixing it!
Tested-by: Christoph Paasch <christoph.paasch@uclouvain.be>
Cheers,
Christoph
>
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 0ab32faa520f..771946487a8d 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2761,7 +2761,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> unsigned int len;
> __be16 proto;
> bool csum;
> - int sg = !!(features & NETIF_F_SG);
> + bool sg = !!(features & NETIF_F_SG);
> int nfrags = skb_shinfo(skb)->nr_frags;
> int err = -ENOMEM;
> int i = 0;
> @@ -2793,7 +2793,11 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> hsize = len;
>
> if (!hsize && i >= nfrags) {
> - BUG_ON(fskb->len != len);
> + if (fskb->len != len) {
> + hsize = len;
> + sg = false;
> + goto do_linear;
> + }
>
> pos += len;
> nskb = skb_clone(fskb, GFP_ATOMIC);
> @@ -2812,6 +2816,7 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> skb_release_head_state(nskb);
> __skb_push(nskb, doffset);
> } else {
> +do_linear:
> nskb = __alloc_skb(hsize + doffset + headroom,
> GFP_ATOMIC, skb_alloc_rx_flag(skb),
> NUMA_NO_NODE);
> @@ -2838,9 +2843,6 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> nskb->data - tnl_hlen,
> doffset + tnl_hlen);
>
> - if (fskb != skb_shinfo(skb)->frag_list)
> - goto perform_csum_check;
> -
> if (!sg) {
> nskb->ip_summed = CHECKSUM_NONE;
> nskb->csum = skb_copy_and_csum_bits(skb, offset,
> @@ -2849,6 +2851,9 @@ struct sk_buff *skb_segment(struct sk_buff *skb, netdev_features_t features)
> continue;
> }
>
> + if (fskb != skb_shinfo(skb)->frag_list)
> + goto perform_csum_check;
> +
> frag = skb_shinfo(nskb)->frags;
>
> skb_copy_from_linear_data_offset(skb, offset,
>
>
^ permalink raw reply
* Re: [PATCH] ethernet/arc/arc_emac: drop redundant mac address check
From: Luka Perkov @ 2013-10-29 9:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, abrodkin
In-Reply-To: <20131028.234842.1406231268962965513.davem@davemloft.net>
Hi David,
On Mon, Oct 28, 2013 at 11:48:42PM -0400, David Miller wrote:
> I've seen you use three inconsistent Subject prefixes here, pick a scheme
> and stick to it please!
>
> First patch was:
>
> netdev: ${driver_name}:
>
> Second patch was:
>
> net: ${driver_name}:
>
> Third patch was:
>
> patch/to/driver/directory:
True. I have sent it that way because I've followed the commit
schematics of each driver individually.
> This is really not acceptable. Just using "${driver_name}: " is good
> enough.
It's not a problem to resend them... But I'm wondering should this be
squashed into one patch?
Luka
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Guennadi Liakhovetski @ 2013-10-29 9:12 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-fbdev, linux-sh, Linus Walleij, Guennadi Liakhovetski,
Thierry Reding, linux-mtd, linux-i2c, Vinod Koul, Joerg Roedel,
Wolfram Sang, Magnus Damm, Eduardo Valentin, Tomi Valkeinen,
linux-serial, linux-input, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard, linux-media, linux-pwm,
Samuel Ortiz, linux-pm, Ian Molton, Mark Brown, linux-arm-kernel,
Ser
In-Reply-To: <1383004027-25036-1-git-send-email-laurent.pinchart+renesas@ideasonboard.com>
Hi Laurent
On Tue, 29 Oct 2013, Laurent Pinchart wrote:
> Hello,
>
> This patch series, based on v3.12-rc7, prepares various Renesas drivers
> for migration to multiplatform kernels by enabling their compilation or
> otherwise fixing them on all ARM platforms. The patches are pretty
> straightforward and are described in their commit message.
>
> I'd like to get all these patches merged in v3.14. As they will need to go
> through their respective subsystems' trees, I would appreciate if all
> maintainers involved could notify me when they merge patches from this series
> in their tree to help me tracking the merge status. I don't plan to send pull
> requests individually for these patches, and I will repost patches
> individually if changes are requested during review.
>
> If you believe the issue should be solved in a different way (for instance by
> removing the architecture dependency completely) please reply to the cover
> letter to let other maintainers chime in.
Exactly this was my doubt. If we let these drivers build on all ARM
platforms... Maybe we should just let them build everywhere? Unless there
are real ARM dependencies. Maybe you could try to remove the restriction
and try to build them all on x86?
Thanks
Guennadi
>
> Cc: Chris Ball <cjb@laptop.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: dmaengine@vger.kernel.org
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Eduardo Valentin <eduardo.valentin@ti.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Guennadi Liakhovetski <g.liakhovetski+renesas@gmail.com>
> Cc: Ian Molton <ian@mnementh.co.uk>
> Cc: iommu@lists.linux-foundation.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Joerg Roedel <joro@8bytes.org>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-i2c@vger.kernel.org
> Cc: linux-input@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-media@vger.kernel.org
> Cc: linux-mmc@vger.kernel.org
> Cc: linux-mtd@lists.infradead.org
> Cc: linux-pm@vger.kernel.org
> Cc: linux-pwm@vger.kernel.org
> Cc: linux-serial@vger.kernel.org
> Cc: linux-spi@vger.kernel.org
> Cc: Magnus Damm <magnus.damm@gmail.com>
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Mauro Carvalho Chehab <m.chehab@samsung.com>
> Cc: netdev@vger.kernel.org
> Cc: Samuel Ortiz <samuel@sortiz.org>
> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: Zhang Rui <rui.zhang@intel.com>
>
> Laurent Pinchart (19):
> serial: sh-sci: Enable the driver on all ARM platforms
> DMA: shdma: Enable the driver on all ARM platforms
> i2c: sh_mobile: Enable the driver on all ARM platforms
> input: sh_keysc: Enable the driver on all ARM platforms
> iommu: shmobile: Enable the driver on all ARM platforms
> i2c: rcar: Enable the driver on all ARM platforms
> v4l: sh_vou: Enable the driver on all ARM platforms
> mmc: sdhi: Enable the driver on all ARM platforms
> mmc: sh_mmcif: Enable the driver on all ARM platforms
> mtd: sh_flctl: Enable the driver on all ARM platforms
> net: sh_eth: Set receive alignment correctly on all ARM platforms
> irda: sh_irda: Enable the driver on all ARM platforms
> pinctrl: sh-pfc: Enable the driver on all ARM platforms
> pwm: pwm-renesas-tpu: Enable the driver on all ARM platforms
> sh: intc: Enable the driver on all ARM platforms
> spi: sh_msiof: Enable the driver on all ARM platforms
> spi: sh_hspi: Enable the driver on all ARM platforms
> thermal: rcar-thermal: Enable the driver on all ARM platforms
> fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms
>
> drivers/dma/sh/Kconfig | 2 +-
> drivers/dma/sh/shdmac.c | 6 +++---
> drivers/i2c/busses/Kconfig | 4 ++--
> drivers/input/keyboard/Kconfig | 2 +-
> drivers/iommu/Kconfig | 2 +-
> drivers/media/platform/Kconfig | 2 +-
> drivers/mmc/host/Kconfig | 4 ++--
> drivers/mmc/host/tmio_mmc_dma.c | 2 +-
> drivers/mtd/nand/Kconfig | 2 +-
> drivers/net/ethernet/renesas/sh_eth.c | 2 +-
> drivers/net/ethernet/renesas/sh_eth.h | 2 +-
> drivers/net/irda/Kconfig | 2 +-
> drivers/pinctrl/Makefile | 2 +-
> drivers/pinctrl/sh-pfc/Kconfig | 2 +-
> drivers/pwm/Kconfig | 2 +-
> drivers/sh/intc/Kconfig | 2 +-
> drivers/spi/Kconfig | 4 ++--
> drivers/thermal/Kconfig | 2 +-
> drivers/tty/serial/Kconfig | 2 +-
> drivers/video/Kconfig | 6 +++---
> 20 files changed, 27 insertions(+), 27 deletions(-)
>
> --
> Regards,
>
> Laurent Pinchart
>
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
^ permalink raw reply
* [PATCH v2 2/2] net/benet: Make lancer_wait_ready() static
From: Gavin Shan @ 2013-10-29 9:30 UTC (permalink / raw)
To: netdev; +Cc: Sathya.Perla, davem, Gavin Shan
In-Reply-To: <1383039057-28164-1-git-send-email-shangw@linux.vnet.ibm.com>
The function needn't to be public, so to make it as static.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 2 +-
drivers/net/ethernet/emulex/benet/be_cmds.h | 1 -
2 files changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 2d55436..7fb0edf 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -522,7 +522,7 @@ static u16 be_POST_stage_get(struct be_adapter *adapter)
return sem & POST_STAGE_MASK;
}
-int lancer_wait_ready(struct be_adapter *adapter)
+static int lancer_wait_ready(struct be_adapter *adapter)
{
#define SLIPORT_READY_TIMEOUT 30
u32 sliport_status;
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 8870837..edf3e8a 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -2053,7 +2053,6 @@ int be_cmd_get_ext_fat_capabilites(struct be_adapter *adapter,
int be_cmd_set_ext_fat_capabilites(struct be_adapter *adapter,
struct be_dma_mem *cmd,
struct be_fat_conf_params *cfgs);
-int lancer_wait_ready(struct be_adapter *adapter);
int lancer_physdev_ctrl(struct be_adapter *adapter, u32 mask);
int lancer_initiate_dump(struct be_adapter *adapter);
bool dump_present(struct be_adapter *adapter);
--
1.7.9.5
^ permalink raw reply related
* [PATCH v2 1/2] net/benet: Remove interface type
From: Gavin Shan @ 2013-10-29 9:30 UTC (permalink / raw)
To: netdev; +Cc: Sathya.Perla, davem, Gavin Shan
The interface type, which is being traced by "struct be_adapter::
if_type", isn't used currently. So we can remove that safely
according to Sathya's comments.
Signed-off-by: Gavin Shan <shangw@linux.vnet.ibm.com>
---
drivers/net/ethernet/emulex/benet/be.h | 1 -
drivers/net/ethernet/emulex/benet/be_main.c | 5 -----
2 files changed, 6 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index b2765eb..2c88ac2 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -470,7 +470,6 @@ struct be_adapter {
u32 rx_fc; /* Rx flow control */
u32 tx_fc; /* Tx flow control */
bool stats_cmd_sent;
- u32 if_type;
struct {
u32 size;
u32 total_size;
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 03e0c74..8080a1a 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4095,11 +4095,6 @@ static int be_roce_map_pci_bars(struct be_adapter *adapter)
static int be_map_pci_bars(struct be_adapter *adapter)
{
u8 __iomem *addr;
- u32 sli_intf;
-
- pci_read_config_dword(adapter->pdev, SLI_INTF_REG_OFFSET, &sli_intf);
- adapter->if_type = (sli_intf & SLI_INTF_IF_TYPE_MASK) >>
- SLI_INTF_IF_TYPE_SHIFT;
if (BEx_chip(adapter) && be_physfn(adapter)) {
adapter->csr = pci_iomap(adapter->pdev, 2, 0);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] net: mvneta: drop redundant mac address check
From: Thomas Petazzoni @ 2013-10-29 9:33 UTC (permalink / raw)
To: Luka Perkov; +Cc: netdev
In-Reply-To: <1383010161-25854-1-git-send-email-luka@openwrt.org>
Dear Luka Perkov,
On Tue, 29 Oct 2013 02:29:21 +0100, Luka Perkov wrote:
> Checking if MAC address is valid using is_valid_ether_addr() is already done in
> of_get_mac_address().
>
> Signed-off-by: Luka Perkov <luka@openwrt.org>
Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v3 4/4] net: mvmdio: doc: mvmdio now used by mv643xx_eth
From: Leigh Brown @ 2013-10-29 9:33 UTC (permalink / raw)
To: David S. Miller
Cc: Leigh Brown, Thomas Petazzoni, Sebastian Hesselbarth, netdev,
linux-arm-kernel, Jason Cooper
In-Reply-To: <cover.1383038973.git.leigh@solinno.co.uk>
Amend the documentation in the mvmdio driver to note the fact
that it is now used by both the mvneta and mv643xx_eth drivers.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
---
drivers/net/ethernet/marvell/mvmdio.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 0cfa0c8..0d0311c 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -4,11 +4,9 @@
* Since the MDIO interface of Marvell network interfaces is shared
* between all network interfaces, having a single driver allows to
* handle concurrent accesses properly (you may have four Ethernet
- * ports, but they in fact share the same SMI interface to access the
- * MDIO bus). Moreover, this MDIO interface code is similar between
- * the mv643xx_eth driver and the mvneta driver. For now, it is only
- * used by the mvneta driver, but it could later be used by the
- * mv643xx_eth driver as well.
+ * ports, but they in fact share the same SMI interface to access
+ * the MDIO bus). This driver is currently used by the mvneta and
+ * mv643xx_eth drivers.
*
* Copyright (C) 2012 Marvell
*
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH v3 0/4] Fix MDIO bus timeout issues on Dreamplug
From: Leigh Brown @ 2013-10-29 9:33 UTC (permalink / raw)
To: David S. Miller
Cc: Leigh Brown, Thomas Petazzoni, Sebastian Hesselbarth, netdev,
linux-arm-kernel, Jason Cooper
In-Reply-To: <cover.1382637156.git.leigh@solinno.co.uk>
I think I have addressed all comments.
Changes since v2:
- Fix coding style in ordio_mdio_wait_ready, as identified by David
- Remove un-needed operator change in orion_mdio_write, as identified by David
This patchset fixes an issue with the Dreamplug MDIO bus timeout since the
mv643xx_eth driver was converted to use the mvmdio driver to access the bus.
Leigh Brown (4):
net: mvmdio: make orion_mdio_wait_ready consistent
net: mvmdio: orion_mdio_ready: remove manual poll
net: mvmdio: slight optimisation of orion_mdio_write
net: mvmdio: doc: mvmdio now used by mv643xx_eth
drivers/net/ethernet/marvell/mvmdio.c | 110 ++++++++++++++++------------------
1 file changed, 53 insertions(+), 57 deletions(-)
--
1.8.4.rc3
^ permalink raw reply
* [PATCH v3 1/4] net: mvmdio: make orion_mdio_wait_ready consistent
From: Leigh Brown @ 2013-10-29 9:33 UTC (permalink / raw)
To: David S. Miller
Cc: Leigh Brown, Thomas Petazzoni, Sebastian Hesselbarth, netdev,
linux-arm-kernel, Jason Cooper
In-Reply-To: <cover.1383038973.git.leigh@solinno.co.uk>
Amend orion_mdio_wait_ready so that the same timeout is used when
polling or using wait_event_timeout. Set the timeout to 1ms.
Replace udelay with usleep_range to avoid a busy loop, and set the
polling interval range as 45us to 55us, so that the first sleep
will be enough in almost all cases.
Generate the same log message at timeout when polling or using
wait_event_timeout.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
---
drivers/net/ethernet/marvell/mvmdio.c | 52 ++++++++++++++++++++---------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index e2f6626..971a4c1 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -44,6 +44,15 @@
#define MVMDIO_ERR_INT_SMI_DONE 0x00000010
#define MVMDIO_ERR_INT_MASK 0x0080
+/*
+ * SMI Timeout measurements:
+ * - Kirkwood 88F6281 (Globalscale Dreamplug): 45us to 95us (Interrupt)
+ * - Armada 370 (Globalscale Mirabox): 41us to 43us (Polled)
+ */
+#define MVMDIO_SMI_TIMEOUT 1000 /* 1000us = 1ms */
+#define MVMDIO_SMI_POLL_INTERVAL_MIN 45
+#define MVMDIO_SMI_POLL_INTERVAL_MAX 55
+
struct orion_mdio_dev {
struct mutex lock;
void __iomem *regs;
@@ -68,34 +77,33 @@ static int orion_mdio_smi_is_done(struct orion_mdio_dev *dev)
static int orion_mdio_wait_ready(struct mii_bus *bus)
{
struct orion_mdio_dev *dev = bus->priv;
- int count;
+ unsigned long timeout = usecs_to_jiffies(MVMDIO_SMI_TIMEOUT);
+ unsigned long end = jiffies + timeout;
+ int timedout = 0;
- if (dev->err_interrupt <= 0) {
- count = 0;
- while (1) {
- if (orion_mdio_smi_is_done(dev))
- break;
+ while (1) {
+ if (orion_mdio_smi_is_done(dev))
+ return 0;
+ else if (timedout)
+ break;
- if (count > 100) {
- dev_err(bus->parent,
- "Timeout: SMI busy for too long\n");
- return -ETIMEDOUT;
- }
+ if (dev->err_interrupt <= 0) {
+ usleep_range(MVMDIO_SMI_POLL_INTERVAL_MIN,
+ MVMDIO_SMI_POLL_INTERVAL_MAX);
- udelay(10);
- count++;
- }
- } else {
- if (!orion_mdio_smi_is_done(dev)) {
+ if (time_is_before_jiffies(end))
+ ++timedout;
+ } else {
wait_event_timeout(dev->smi_busy_wait,
- orion_mdio_smi_is_done(dev),
- msecs_to_jiffies(100));
- if (!orion_mdio_smi_is_done(dev))
- return -ETIMEDOUT;
- }
+ orion_mdio_smi_is_done(dev),
+ timeout);
+
+ ++timedout;
+ }
}
- return 0;
+ dev_err(bus->parent, "Timeout: SMI busy for too long\n");
+ return -ETIMEDOUT;
}
static int orion_mdio_read(struct mii_bus *bus, int mii_id,
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH v3 2/4] net: mvmdio: orion_mdio_ready: remove manual poll
From: Leigh Brown @ 2013-10-29 9:33 UTC (permalink / raw)
To: David S. Miller
Cc: Leigh Brown, Thomas Petazzoni, Sebastian Hesselbarth, netdev,
linux-arm-kernel, Jason Cooper
In-Reply-To: <cover.1383038973.git.leigh@solinno.co.uk>
Replace manual poll of MVMDIO_SMI_READ_VALID with a call to
orion_mdio_wait_ready. This ensures a consistent timeout,
eliminates a busy loop, and allows for use of interrupts on
systems that support them.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
---
drivers/net/ethernet/marvell/mvmdio.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index 971a4c1..e3898b3 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -110,43 +110,35 @@ static int orion_mdio_read(struct mii_bus *bus, int mii_id,
int regnum)
{
struct orion_mdio_dev *dev = bus->priv;
- int count;
u32 val;
int ret;
mutex_lock(&dev->lock);
ret = orion_mdio_wait_ready(bus);
- if (ret < 0) {
- mutex_unlock(&dev->lock);
- return ret;
- }
+ if (ret < 0)
+ goto out;
writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
MVMDIO_SMI_READ_OPERATION),
dev->regs);
- /* Wait for the value to become available */
- count = 0;
- while (1) {
- val = readl(dev->regs);
- if (val & MVMDIO_SMI_READ_VALID)
- break;
-
- if (count > 100) {
- dev_err(bus->parent, "Timeout when reading PHY\n");
- mutex_unlock(&dev->lock);
- return -ETIMEDOUT;
- }
+ ret = orion_mdio_wait_ready(bus);
+ if (ret < 0)
+ goto out;
- udelay(10);
- count++;
+ val = readl(dev->regs);
+ if (!(val & MVMDIO_SMI_READ_VALID)) {
+ dev_err(bus->parent, "SMI bus read not valid\n");
+ ret = -ENODEV;
+ goto out;
}
+ ret = val & 0xFFFF;
+out:
mutex_unlock(&dev->lock);
-
- return val & 0xFFFF;
+ return ret;
}
static int orion_mdio_write(struct mii_bus *bus, int mii_id,
--
1.8.4.rc3
^ permalink raw reply related
* [PATCH v3 3/4] net: mvmdio: slight optimisation of orion_mdio_write
From: Leigh Brown @ 2013-10-29 9:33 UTC (permalink / raw)
To: David S. Miller
Cc: Leigh Brown, Thomas Petazzoni, Sebastian Hesselbarth, netdev,
linux-arm-kernel, Jason Cooper
In-Reply-To: <cover.1383038973.git.leigh@solinno.co.uk>
Make only a single call to mutex_unlock in orion_mdio_write.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
---
drivers/net/ethernet/marvell/mvmdio.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvmdio.c b/drivers/net/ethernet/marvell/mvmdio.c
index e3898b3..0cfa0c8 100644
--- a/drivers/net/ethernet/marvell/mvmdio.c
+++ b/drivers/net/ethernet/marvell/mvmdio.c
@@ -150,10 +150,8 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
mutex_lock(&dev->lock);
ret = orion_mdio_wait_ready(bus);
- if (ret < 0) {
- mutex_unlock(&dev->lock);
- return ret;
- }
+ if (ret < 0)
+ goto out;
writel(((mii_id << MVMDIO_SMI_PHY_ADDR_SHIFT) |
(regnum << MVMDIO_SMI_PHY_REG_SHIFT) |
@@ -161,9 +159,9 @@ static int orion_mdio_write(struct mii_bus *bus, int mii_id,
(value << MVMDIO_SMI_DATA_SHIFT)),
dev->regs);
+out:
mutex_unlock(&dev->lock);
-
- return 0;
+ return ret;
}
static int orion_mdio_reset(struct mii_bus *bus)
--
1.8.4.rc3
^ permalink raw reply related
* Re: [PATCH 1/4 net-next] net: phy: add Generic Netlink Ethernet switch configuration API
From: Felix Fietkau @ 2013-10-29 9:34 UTC (permalink / raw)
To: Jamal Hadi Salim, Florian Fainelli, Neil Horman
Cc: John Fastabend, netdev, David Miller, Sascha Hauer, John Crispin,
Jonas Gorski, Gary Thomas, Vlad Yasevich, Stephen Hemminger
In-Reply-To: <526EEAE9.6090508@mojatatu.com>
On 2013-10-28 23:53, Jamal Hadi Salim wrote:
> On 10/27/13 15:51, Felix Fietkau wrote:
>> On 2013-10-27 6:19 PM, Jamal Hadi Salim wrote:
>
>> That question does not make any sense to me. Aside from low level
>> control frames like pause frames for flow control, the switch has no
>> need to send packets to the CPU port on its own.
>> Remember what I told you about the switch being a *separate* entity from
>> the NIC that connects it to the CPU.
>>
>
> I am assuming there is a MAC address which is identified to be that of a
> switch. Something responds to ARP for example for that MAC. I think
> you are saying that for a certain class of switch chips, there is no
> concept of "cpu port" - therefore there cannot be unicast from the
> chip to the cpu.
These are simple switches, why would they respond to ARP?
I suspect that you're attributing too much functionality to the switch
itself. Think of it as a device similar to the cheap unmanaged ones you
can buy in a shop and hook up to your machine via Ethernet.
Add to that some very limited VLAN grouping functionality, and you're
pretty close to the limits of what these switches can do.
They don't do ARP, IP or other things. They learn about MAC addresses
from incoming packets to build their forwarding path.
The CPU port in this case is whatever port on the switch that you plug
the cable of your machine into :)
>> One of the currently very common switches in many embedded devices is
>> the RTL8366/RTL8367. It has some flexibility when it comes to
>> configuring VLANs, and it's one of the few ones where you can configure
>> a forwarding table for a VLAN (which spans multiple ports), which allows
>> software bridging between multiple VLANs.
>> However, what this switch does *not* support is adding a header/trailer
>> to packets to indicate the originating port.
>> This means that all per-port netdevs will be dummy ports which don't
>> include the data path.
>
> My view is that netdevs are still valuable even if only they get used
> for control path. Like you said earlier - you can still pull stats, flow
> control messages still make it through etc. They provide you
> the consistent api to configure the switch above, ex:
> If i was to use the FDB api for this switch as long as i can
> abstract it in software as a bridge, I could send it a switch config
> via its ops which says:
> "I am giving you this entry with vland 400 for port 2, but i want you to
> send it to the hardware not to your local entry"
The FDB related abstraction that you're describing will not work with
the hardware that I'm talking about. Let's leave that one out of this
discussion.
As for per-port netdevs: Yes, you could pull stats.
No, flow control messages would not make it through.
No idea how it would provide a *consistent* API.
Either way, if adding netdevs just for stats and link state, that could
be easily added on top of swconfig (or whatever name we pick for it)
later. I just don't think it's worth it at this point.
>> So let's say you have a configuration where you're using VLAN ID 4 on
>> port 1, and you want to bridge it to VLAN ID 400 on port 2.
>>
>> Sounds easy enough, you can easily create a bridge that spans port1.4
>> and port2.400. Except, this particular switch (like pretty much any
>> other switch supported by swconfig) isn't actually able to handle such a
>> configuration on its own.
>
> Makes sense.
> Let me point that even the Linux bridge cant handle this on its own
> either.
> You would need two bridges instantiated. The "cpu port" (we should call
> it the "L3 port" really) is implicit in the case of the bridge i.e it
> is the Linux network stack.
> You would need to set the vlan filters on the bridge to strip the vlan
> on egress of the first bridge etc ..
>
>> It needs two VLAN configurations, with different forwarding table IDs,
>> and then the software bridge on the CPU port needs to forward between
>> the two different VLANs.
>> To be able to handle such a configuration, the code would have to detect
>> this kind of special case scenario, somehow hook itself via rx handler
>> into the NIC connected to the CPU port and emulate that VLAN ID
>> replacement behavior.
>
> IMO: You dont need to muck with rx handler if you used bridge
> abstraction. It becomes a config issue.
If we don't need to muck with an rx handler, how are packets intercepted
from the NIC that connects to the switch?
That NIC is run by a driver that knows nothing about switch stuff.
>> With swconfig, you create two VLANs: VLAN 4, containing CPU and port1;
>> VLAN 400, containing CPU and port2. You then create a software bridge
>> between eth0.4 and eth0.400 (assuming eth0 is the NIC connected to the
>> switch).
>
> Can we call that "L3" instead of software bridge?
L3? Why?
> Understood.
> I think that discovery is a must - so you can apply different behavior
> to different switches.
> But you seem to have solved this already. Linux as is does not.
> You can either have the driver tell you what it can/cant do or you
> can attempt to fire and miss and get a return code that will tell
> you that it cant achieve what you are asking it to do. I prefer the
> former.
I think that's way more confusing to users than presenting a consistent
model that properly reflects what you can do with the hardware.
But I sense a pattern here. I've long had my beef with quite a few Linux
network related APIs for being inconsistent, having no decent error
reporting when you're trying to configure things (errno doesn't count,
it's just too ambiguous), and just making it hard to figure out the
capabilities. Of course, none of this can be easily fixed due to ABI
stability constraints.
I do NOT wish to follow that pattern!
>> Those are just two simple scenarios from the top of my head - I'm pretty
>> sure I could come up with a long list of further corner cases and
>> quirks, which are simply either difficult to deal with, or completely
>> unnatural in the model that you're describing.
> I think these are the kind of things that need to be enumerated to come
> to some conclusion.
I'm not going to try to enumerate all the case; I have other projects
that I need to work on. :)
>> Trying to make all of these cases work in the code will make the whole
>> thing a lot more difficult to deal with and maintain. It will also make
>> it much harder for the user to figure out, what configurations work, and
>> what configurations don't.
>>
>>
>> Especially the case with reusing VLANs on different ports (but not
>> connecting them to each other) is something that can easily work with
>> software devices, but cannot be emulated on most embedded device
>> switches. The software bridge configuration model raises a lot of
>> expectations that these switches simply cannot meet.
> I wouldnt expect every thing a software bridge does would be met by
> a random switch.S/w bridge would be the super-set. But this is
> not a new concept, example: Netdev itself is an abstraction - we have
> USB, ethernet, wireless, variety of virtual interfaces etc.
> Sometimes we dont even have the concept of a "link" in some of these
> devices; infiniband would have a huge MAC address but i can still
> use ifconfig on it etc.
Only a *tiny* part of the software bridge configuration model can be
emulated, the rest does not fit and has to be handled through extensions
or different APIs anyway. That's why I am convinced that it's a really
bad model to try to make these switches fit into it.
You gain a tiny advantage with writing scripts, but at the same time,
the code gets more complex, the configuration interface gets more
confusing, there are more nasty corner cases to take care of.
Why do you insist on making so many things worse just for one tiny
advantage? Where's the pragmatic cost/benefit tradeoff?
>> If you look at the swconfig model, you will see that the abstraction
>> clearly communicates the limitations of these typical switches.
>>
>
> I will have to go back and look - but like i said earlier seems to me
> you have solved this problem. Of the switch hardware i am familiar with
> (high end pricey stuff), the capabilities tend to fall into the
> following components:
> -flooding control (i.e what should happen on destination failure)
> -learning control (i.e what should happen on the source lookup failure)
> (Ive seen knobs for "drop", "send to portX" where "X" could be cpu etc)
> -fdb capacity
> -whether it can do vlans, filtering pvids etc
> -multicast snooping capability
Right, with most of the switches that we support, almost none of these
things work in a way that can be integrated with the network stack.
> To add to the above a few more based on talking to you:
> - cpu port (in what ive come across this is always present, but
> as you point out this cannot be assumed)
I'm not even sure what you mean when you say 'cpu port cannot be
assumed'. On pretty much all devices that we work with, one of the ports
connects to a NIC in the CPU. It's just that the switch cannot be
assumed to have special treatment for that CPU port. As far as it is
concerned, it is just another port like the others.
> - ingress port tag (you point out that some cases this may never be
> present even when the cpu port is present)
> - ive never seen table id, but i think this is another one; in which
> case the number of table ids becomes something one needs to discover..
Yes, and this is something that doesn't even map directly to something
in the software bridge world.
- Felix
^ permalink raw reply
* RE: [patch net-next] ipv6: allow userspace to create address with IFLA_F_TEMPORARY flag
From: David Laight @ 2013-10-29 9:37 UTC (permalink / raw)
To: David Miller, hannes
Cc: jiri, vyasevich, netdev, kuznet, jmorris, yoshfuji, kaber,
thaller, stephen
In-Reply-To: <20131028.204306.2213130677400093266.davem@davemloft.net>
> Note that you don't even need to put the DHCP protocol core into the
> kernel to fix the promiscuous problem. You just have to use the
> current kernel interfaces correctly.
>
> It used to be the case a very long time ago that you couldn't even
> receive broadcast UDP datagrams on a socket until an address was
> configured on it.
>
> So everyone turns on promiscuous mode and uses RAW sockets or
> AF_PACKET.
>
> Stupid? yes.
Not only that, but the dhcp client could use a normal UDP socket
to keep the lease renewed - I suspect it has only ever needed
to use the BPF interface (I didn't think it set promiscuous)
when acquiring the initial lease.
David
^ permalink raw reply
* Re: [PATCH 00/19] Enable various Renesas drivers on all ARM platforms
From: Laurent Pinchart @ 2013-10-29 9:46 UTC (permalink / raw)
To: Guennadi Liakhovetski
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-sh-u79uwXL29TY76Z2rM5mHXA, Linus Walleij,
Guennadi Liakhovetski, Thierry Reding,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA, Laurent Pinchart, Vinod Koul,
Wolfram Sang, Magnus Damm, Eduardo Valentin, Tomi Valkeinen,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Zhang Rui, Chris Ball,
Jean-Christophe Plagniol-Villard,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pwm-u79uwXL29TY76Z2rM5mHXA, Samuel Ortiz,
linux-pm-u79uwXL29TY76Z2rM5mHXA, Ian Molton, Mark Brown,
linux-arm-k
In-Reply-To: <Pine.LNX.4.64.1310291009121.8404-0199iw4Nj15frtckUFj5Ag@public.gmane.org>
Hi Simon and Guennadi,
On Tuesday 29 October 2013 10:12:17 Guennadi Liakhovetski wrote:
> On Tue, 29 Oct 2013, Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series, based on v3.12-rc7, prepares various Renesas drivers
> > for migration to multiplatform kernels by enabling their compilation or
> > otherwise fixing them on all ARM platforms. The patches are pretty
> > straightforward and are described in their commit message.
> >
> > I'd like to get all these patches merged in v3.14. As they will need to go
> > through their respective subsystems' trees, I would appreciate if all
> > maintainers involved could notify me when they merge patches from this
> > series in their tree to help me tracking the merge status. I don't plan
> > to send pull requests individually for these patches, and I will repost
> > patches individually if changes are requested during review.
> >
> > If you believe the issue should be solved in a different way (for instance
> > by removing the architecture dependency completely) please reply to the
> > cover letter to let other maintainers chime in.
>
> Exactly this was my doubt. If we let these drivers build on all ARM
> platforms... Maybe we should just let them build everywhere? Unless there
> are real ARM dependencies. Maybe you could try to remove the restriction
> and try to build them all on x86?
My concern is that they might not build on some architectures for which I
don't have a cross-compiler. For instance not all architecture define ioread32
and iowrite32, which some drivers might rely on. What about depending on
SUPERH || ARM || COMPILE_TEST to start with, in order not to push compilation
breakages to mainline ?
> > Cc: Chris Ball <cjb-2X9k7bc8m7Mdnm+yROfE0A@public.gmane.org>
> > Cc: "David S. Miller" <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
> > Cc: David Woodhouse <dwmw2-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > Cc: dmaengine-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Eduardo Valentin <eduardo.valentin-l0cyMroinI0@public.gmane.org>
> > Cc: Greg Kroah-Hartman <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> > Cc: Guennadi Liakhovetski <g.liakhovetski+renesas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Ian Molton <ian-zdned+2MO1+9FHfhHBbuYA@public.gmane.org>
> > Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj-sclMFOaUSTBWk0Htik3J/w@public.gmane.org>
> > Cc: Joerg Roedel <joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org>
> > Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> > Cc: linux-pm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-pwm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-spi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Magnus Damm <magnus.damm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Mauro Carvalho Chehab <m.chehab-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Samuel Ortiz <samuel-jcdQHdrhKHMdnm+yROfE0A@public.gmane.org>
> > Cc: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
> > Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> > Cc: Vinod Koul <vinod.koul-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
> > Cc: Zhang Rui <rui.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> >
> > Laurent Pinchart (19):
> > serial: sh-sci: Enable the driver on all ARM platforms
> > DMA: shdma: Enable the driver on all ARM platforms
> > i2c: sh_mobile: Enable the driver on all ARM platforms
> > input: sh_keysc: Enable the driver on all ARM platforms
> > iommu: shmobile: Enable the driver on all ARM platforms
> > i2c: rcar: Enable the driver on all ARM platforms
> > v4l: sh_vou: Enable the driver on all ARM platforms
> > mmc: sdhi: Enable the driver on all ARM platforms
> > mmc: sh_mmcif: Enable the driver on all ARM platforms
> > mtd: sh_flctl: Enable the driver on all ARM platforms
> > net: sh_eth: Set receive alignment correctly on all ARM platforms
> > irda: sh_irda: Enable the driver on all ARM platforms
> > pinctrl: sh-pfc: Enable the driver on all ARM platforms
> > pwm: pwm-renesas-tpu: Enable the driver on all ARM platforms
> > sh: intc: Enable the driver on all ARM platforms
> > spi: sh_msiof: Enable the driver on all ARM platforms
> > spi: sh_hspi: Enable the driver on all ARM platforms
> > thermal: rcar-thermal: Enable the driver on all ARM platforms
> > fbdev: sh-mobile-lcdcfb: Enable the driver on all ARM platforms
> >
> > drivers/dma/sh/Kconfig | 2 +-
> > drivers/dma/sh/shdmac.c | 6 +++---
> > drivers/i2c/busses/Kconfig | 4 ++--
> > drivers/input/keyboard/Kconfig | 2 +-
> > drivers/iommu/Kconfig | 2 +-
> > drivers/media/platform/Kconfig | 2 +-
> > drivers/mmc/host/Kconfig | 4 ++--
> > drivers/mmc/host/tmio_mmc_dma.c | 2 +-
> > drivers/mtd/nand/Kconfig | 2 +-
> > drivers/net/ethernet/renesas/sh_eth.c | 2 +-
> > drivers/net/ethernet/renesas/sh_eth.h | 2 +-
> > drivers/net/irda/Kconfig | 2 +-
> > drivers/pinctrl/Makefile | 2 +-
> > drivers/pinctrl/sh-pfc/Kconfig | 2 +-
> > drivers/pwm/Kconfig | 2 +-
> > drivers/sh/intc/Kconfig | 2 +-
> > drivers/spi/Kconfig | 4 ++--
> > drivers/thermal/Kconfig | 2 +-
> > drivers/tty/serial/Kconfig | 2 +-
> > drivers/video/Kconfig | 6 +++---
> > 20 files changed, 27 insertions(+), 27 deletions(-)
--
Regards,
Laurent Pinchart
^ permalink raw reply
* IPV6 nf defrag does not work
From: Jiri Pirko @ 2013-10-29 10:52 UTC (permalink / raw)
To: netdev; +Cc: pablo, netfilter-devel, yoshfuji, kadlec, kaber
Hi All.
On the current net-next if you on HOSTA do:
ip6tables -I INPUT -p icmpv6 -j DROP
ip6tables -I INPUT -p icmpv6 -m icmp6 --icmpv6-type 128 -j ACCEPT
and on HOSTB you do:
ping6 HOSTA -s2000 (MTU is 1500)
Only the first ICMP echo request will be passed through, the rest is not
passed on HOSTA. This issue does not occur with smaller packets than MTU (where
fragmentation does not happen).
I'm trying to find out where the problem is.
Any quick ideas?
Thanks
Jiri
^ permalink raw reply
* [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
From: Ying Xue @ 2013-10-29 11:01 UTC (permalink / raw)
To: davem, maloy; +Cc: jon.maloy, netdev, Paul.Gortmaker, tipc-discussion
The message dispatching part of tipc_recv_msg() is wrapped layers of
while/if/if/switch, causing out-of-control indentation and does not
look very good. We reduce two indentation levels by separating the
message dispatching from the blocks that checks link state and
sequence numbers, allowing longer function and arg names to be
consistently indented without wrapping. This is a cosmetic change
that does not alter the operation of TIPC in any way.
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Reviewed-by: Erik Hugne <erik.hugne@ericsson.com>
---
net/tipc/link.c | 147 +++++++++++++++++++++++++++----------------------------
1 file changed, 72 insertions(+), 75 deletions(-)
diff --git a/net/tipc/link.c b/net/tipc/link.c
index e8153f6..1bfee8c 100644
--- a/net/tipc/link.c
+++ b/net/tipc/link.c
@@ -1593,97 +1593,94 @@ void tipc_recv_msg(struct sk_buff *head, struct tipc_bearer *b_ptr)
/* Now (finally!) process the incoming message */
protocol_check:
- if (likely(link_working_working(l_ptr))) {
- if (likely(seq_no == mod(l_ptr->next_in_no))) {
- l_ptr->next_in_no++;
- if (unlikely(l_ptr->oldest_deferred_in))
- head = link_insert_deferred_queue(l_ptr,
- head);
-deliver:
- if (likely(msg_isdata(msg))) {
- tipc_node_unlock(n_ptr);
- tipc_port_recv_msg(buf);
- continue;
- }
- switch (msg_user(msg)) {
- int ret;
- case MSG_BUNDLER:
- l_ptr->stats.recv_bundles++;
- l_ptr->stats.recv_bundled +=
- msg_msgcnt(msg);
- tipc_node_unlock(n_ptr);
- tipc_link_recv_bundle(buf);
- continue;
- case NAME_DISTRIBUTOR:
- n_ptr->bclink.recv_permitted = true;
- tipc_node_unlock(n_ptr);
- tipc_named_recv(buf);
- continue;
- case BCAST_PROTOCOL:
- tipc_link_recv_sync(n_ptr, buf);
- tipc_node_unlock(n_ptr);
- continue;
- case CONN_MANAGER:
- tipc_node_unlock(n_ptr);
- tipc_port_recv_proto_msg(buf);
- continue;
- case MSG_FRAGMENTER:
- l_ptr->stats.recv_fragments++;
- ret = tipc_link_recv_fragment(
- &l_ptr->defragm_buf,
- &buf, &msg);
- if (ret == 1) {
- l_ptr->stats.recv_fragmented++;
- goto deliver;
- }
- if (ret == -1)
- l_ptr->next_in_no--;
- break;
- case CHANGEOVER_PROTOCOL:
- type = msg_type(msg);
- if (link_recv_changeover_msg(&l_ptr,
- &buf)) {
- msg = buf_msg(buf);
- seq_no = msg_seqno(msg);
- if (type == ORIGINAL_MSG)
- goto deliver;
- goto protocol_check;
- }
- break;
- default:
- kfree_skb(buf);
- buf = NULL;
- break;
- }
+ if (unlikely(!link_working_working(l_ptr))) {
+ if (msg_user(msg) == LINK_PROTOCOL) {
+ link_recv_proto_msg(l_ptr, buf);
+ head = link_insert_deferred_queue(l_ptr, head);
+ tipc_node_unlock(n_ptr);
+ continue;
+ }
+
+ /* Traffic message. Conditionally activate link */
+ link_state_event(l_ptr, TRAFFIC_MSG_EVT);
+
+ if (link_working_working(l_ptr)) {
+ /* Re-insert buffer in front of queue */
+ buf->next = head;
+ head = buf;
tipc_node_unlock(n_ptr);
- tipc_net_route_msg(buf);
continue;
}
+ tipc_node_unlock(n_ptr);
+ goto cont;
+ }
+
+ /* Link is now in state WORKING_WORKING */
+ if (unlikely(seq_no != mod(l_ptr->next_in_no))) {
link_handle_out_of_seq_msg(l_ptr, buf);
head = link_insert_deferred_queue(l_ptr, head);
tipc_node_unlock(n_ptr);
continue;
}
-
- /* Link is not in state WORKING_WORKING */
- if (msg_user(msg) == LINK_PROTOCOL) {
- link_recv_proto_msg(l_ptr, buf);
+ l_ptr->next_in_no++;
+ if (unlikely(l_ptr->oldest_deferred_in))
head = link_insert_deferred_queue(l_ptr, head);
+deliver:
+ if (likely(msg_isdata(msg))) {
tipc_node_unlock(n_ptr);
+ tipc_port_recv_msg(buf);
continue;
}
-
- /* Traffic message. Conditionally activate link */
- link_state_event(l_ptr, TRAFFIC_MSG_EVT);
-
- if (link_working_working(l_ptr)) {
- /* Re-insert buffer in front of queue */
- buf->next = head;
- head = buf;
+ switch (msg_user(msg)) {
+ int ret;
+ case MSG_BUNDLER:
+ l_ptr->stats.recv_bundles++;
+ l_ptr->stats.recv_bundled += msg_msgcnt(msg);
+ tipc_node_unlock(n_ptr);
+ tipc_link_recv_bundle(buf);
+ continue;
+ case NAME_DISTRIBUTOR:
+ n_ptr->bclink.recv_permitted = true;
+ tipc_node_unlock(n_ptr);
+ tipc_named_recv(buf);
+ continue;
+ case BCAST_PROTOCOL:
+ tipc_link_recv_sync(n_ptr, buf);
+ tipc_node_unlock(n_ptr);
+ continue;
+ case CONN_MANAGER:
tipc_node_unlock(n_ptr);
+ tipc_port_recv_proto_msg(buf);
continue;
+ case MSG_FRAGMENTER:
+ l_ptr->stats.recv_fragments++;
+ ret = tipc_link_recv_fragment(&l_ptr->defragm_buf,
+ &buf, &msg);
+ if (ret == 1) {
+ l_ptr->stats.recv_fragmented++;
+ goto deliver;
+ }
+ if (ret == -1)
+ l_ptr->next_in_no--;
+ break;
+ case CHANGEOVER_PROTOCOL:
+ type = msg_type(msg);
+ if (link_recv_changeover_msg(&l_ptr, &buf)) {
+ msg = buf_msg(buf);
+ seq_no = msg_seqno(msg);
+ if (type == ORIGINAL_MSG)
+ goto deliver;
+ goto protocol_check;
+ }
+ break;
+ default:
+ kfree_skb(buf);
+ buf = NULL;
+ break;
}
tipc_node_unlock(n_ptr);
+ tipc_net_route_msg(buf);
+ continue;
cont:
kfree_skb(buf);
}
--
1.7.9.5
------------------------------------------------------------------------------
Android is increasing in popularity, but the open development platform that
developers love is also attractive to malware creators. Download this white
paper to learn more about secure code signing practices that can help keep
Android apps secure.
http://pubads.g.doubleclick.net/gampad/clk?id=65839951&iu=/4140/ostg.clktrk
^ permalink raw reply related
* Re: [PATCH] bridge: pass correct vlan id to multicast code
From: Toshiaki Makita @ 2013-10-29 11:08 UTC (permalink / raw)
To: Amos Kong; +Cc: Vlad Yasevich, netdev, shemminger
In-Reply-To: <20131029023646.GA2795@amosk.info>
On Tue, 2013-10-29 at 10:36 +0800, Amos Kong wrote:
> On Mon, Oct 28, 2013 at 03:45:07PM -0400, Vlad Yasevich wrote:
> > Currently multicast code attempts to extrace the vlan id from
> > the skb even when vlan filtering is disabled. This can lead
> > to mdb entries being created with the wrong vlan id.
> > Pass the already extracted vlan id to the multicast
> > filtering code to make the correct id is used in
> > creation as well as lookup.
Thanks!
Acked-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
>
> Hi Vlad,
>
> Can we just update br_vlan_get_tag() to set vid to 0 if dev->vlan is
> disabled? I guess it would effect br_handle_local_finish().
br_handle_local_finish() looks also buggy.
But adding vlan enabled checking would not fix it completely because
vlan_bitmap and PVID are not taken into account in that function.
Since we cannot pass vid as an argument from br_dev_xmit() to
br_handle_[local/frame]_finish() because of NF_HOOK,
br_handle_local_finish() seems to have to check vlan_enabled,
vlan_bitmap, and pvid by itself.
IMHO it can be addressed by another patch.
>
> > Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> > ---
> > net/bridge/br_device.c | 2 +-
> > net/bridge/br_input.c | 2 +-
> > net/bridge/br_multicast.c | 44 +++++++++++++++++++-------------------------
> > net/bridge/br_private.h | 6 ++++--
> > 4 files changed, 25 insertions(+), 29 deletions(-)
>
> ...
>
> > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> > index 8b0b610..686284f 100644
> > --- a/net/bridge/br_multicast.c
> > +++ b/net/bridge/br_multicast.c
> > @@ -947,7 +947,8 @@ void br_multicast_disable_port(struct net_bridge_port *port)
> >
> > static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
> > struct net_bridge_port *port,
> > - struct sk_buff *skb)
> > + struct sk_buff *skb,
> > + u16 vid)
> > {
> > struct igmpv3_report *ih;
> > struct igmpv3_grec *grec;
> > @@ -957,12 +958,10 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
> > int type;
> > int err = 0;
> > __be32 group;
> > - u16 vid = 0;
> >
> > if (!pskb_may_pull(skb, sizeof(*ih)))
> > return -EINVAL;
> >
> > - br_vlan_get_tag(skb, &vid);
>
> After applied the patch, we always use vid in br_dev_xmit()->br_allowed_ingress(),
> is it possible that the vlan of bridge is re-enabled when other
> changed functions are called?
>
> We can just add a enabled checking before this kind of br_vlan_get_tag()?
>
> if (!br->vlan_enabled)
> br_vlan_get_tag(skb2, &vid);
Maybe this leads to a wrong way to update mdb in some cases like
Vlan_filtering is disabled (by default).
Add some vids we want to allow.
Receive a frame whose vid wouldn't be allowed with vlan_filtering enabled.
The frame passes br_allowed_ingress().
Enable vlan_filtering.
The frame reaches br_ip4_multicast_igmp3_report().
Mdb is updated with disabled vid.
Thanks,
Toshiaki Makita
>
>
> > ih = igmpv3_report_hdr(skb);
> > num = ntohs(ih->ngrec);
> > len = sizeof(*ih);
>
> ...
>
^ permalink raw reply
* RE: [PATCH net-next] tipc: remove two indentation levels in tipc_recv_msg routine
From: David Laight @ 2013-10-29 11:13 UTC (permalink / raw)
To: Ying Xue, davem, maloy
Cc: Paul.Gortmaker, jon.maloy, erik.hugne, tipc-discussion, netdev
In-Reply-To: <1383044471-12982-1-git-send-email-ying.xue@windriver.com>
> The message dispatching part of tipc_recv_msg() is wrapped layers of
> while/if/if/switch, causing out-of-control indentation and does not
> look very good. We reduce two indentation levels by separating the
> message dispatching from the blocks that checks link state and
> sequence numbers, allowing longer function and arg names to be
> consistently indented without wrapping. This is a cosmetic change
> that does not alter the operation of TIPC in any way.
...
> + tipc_node_unlock(n_ptr);
> + goto cont;
> + }
...
> continue;
> cont:
> kfree_skb(buf);
> }
I can only see one 'goto cont', an explicit kfree_skb() and
continue would be clearer.
David
^ 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