* Re: [PATCH 2/3] net: ethernet: cpsw-phy-sel: Remove redundant of_match_ptr
From: Mugunthan V N @ 2013-09-30 15:47 UTC (permalink / raw)
To: Sachin Kamat, netdev; +Cc: davem
In-Reply-To: <1380515114-2823-2-git-send-email-sachin.kamat@linaro.org>
On Sunday 29 September 2013 11:25 PM, Sachin Kamat wrote:
> The data structure of_match_ptr() protects is always compiled in.
> Hence of_match_ptr() is not needed.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH 1/3] net: ethernet: cpsw: Remove redundant of_match_ptr
From: Mugunthan V N @ 2013-09-30 15:47 UTC (permalink / raw)
To: Sachin Kamat, netdev; +Cc: davem
In-Reply-To: <1380515114-2823-1-git-send-email-sachin.kamat@linaro.org>
On Sunday 29 September 2013 11:25 PM, Sachin Kamat wrote:
> The data structure of_match_ptr() protects is always compiled in.
> Hence of_match_ptr() is not needed.
>
> Signed-off-by: Sachin Kamat <sachin.kamat@linaro.org>
Acked-by: Mugunthan V N <mugunthanvnm@ti.com>
Regards
Mugunthan V N
^ permalink raw reply
* Re: [PATCH v4] net/hsr: Add support for the High-availability Seamless Redundancy protocol (HSRv0)
From: Arvid Brodin @ 2013-09-30 15:09 UTC (permalink / raw)
To: David Miller
Cc: netdev, shemminger, joe, jboticario, balferreira, elias.molina
In-Reply-To: <20130920.151017.2125552741552759738.davem@davemloft.net>
On 2013-09-20 21:10, David Miller wrote:
> From: Arvid Brodin <arvid.brodin@xdin.com>
> Date: Thu, 19 Sep 2013 03:11:58 +0200
>
>> +#if !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
>> + /* We need to memmove the whole header to work around
>> + * alignment problems caused by the 6-byte HSR tag.
>> + */
>> + memmove(skb_deliver->data - HSR_TAGLEN, skb_deliver->data,
>> + skb_headlen(skb_deliver));
>> + skb_deliver->data -= HSR_TAGLEN;
>> + skb_deliver->tail -= HSR_TAGLEN;
>> +#endif
>
> You can't do this.
>
> First of all, you have no idea if subtracting skb->data a given amount
> will underflow the skb buffer start. You aren't even checking, all
> of the standard skb_*() data adjustment interfaces do.
(Shorter and more to the point than my previous replies:)
I _do_ know: this can't possibly underflow since strip_hsr_tag() a
few lines above pulled the same amount of data. I will rename
strip_hsr_tag() to hsr_pull_tag() to make this clearer.
> Secondly, everything after the header is now at the wrong offset from
> the beginning of the packet.
How does this matter? The memmove moves everything back (restores the
changes made to the packet on the sending side) so that it is at the
"normal" position for an ethernet packet.
--
Arvid Brodin | Consultant (Linux)
XDIN AB | Knarrarnäsgatan 7 | SE-164 40 Kista | Sweden | xdin.com
^ permalink raw reply
* Re: [Xen-devel] [PATCH net-next] xen-netfront: convert to GRO API and advertise this feature
From: Konrad Rzeszutek Wilk @ 2013-09-30 14:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: David Miller, netdev, wei.liu2, abchak, xen-devel
In-Reply-To: <1380532331.1302.9.camel@kazak.uk.xensource.com>
On Mon, Sep 30, 2013 at 10:12:11AM +0100, Ian Campbell wrote:
> On Sat, 2013-09-28 at 15:38 -0400, David Miller wrote:
> > From: Wei Liu <wei.liu2@citrix.com>
> > Date: Sat, 21 Sep 2013 17:05:43 +0100
> >
> > > @@ -1371,7 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
> > > netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> > > netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> > > NETIF_F_GSO_ROBUST;
> > > - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
> > > + netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO |
> > > + NETIF_F_GRO;
> >
> > Please post a new version of this patch with the feedback you've been
> > given integrated, in particular with this part removed because it is
> > not necessary.
> >
> > Ian, please review the patch when Wei posts it.
>
> I will, but note:
> $ ./scripts/get_maintainer.pl -f drivers/net/xen-netfront.c
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> (supporter:XEN HYPERVISOR IN...)
> Jeremy Fitzhardinge <jeremy@goop.org> (supporter:XEN HYPERVISOR IN...)
> xen-devel@lists.xensource.com (moderated list:XEN HYPERVISOR IN...)
> virtualization@lists.linux-foundation.org (open list:XEN HYPERVISOR IN...)
> netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
> linux-kernel@vger.kernel.org (open list)
>
> Strictly speaking I maintain netback not front, so Wei please remember
> to CC the right people (mainly Konrad) as well as me.
Which was Acked:
http://mid.gmane.org/20130924163036.GB13979@phenom.dumpdata.com
>
> BTW I think this separation is a good thing since it keeps changes to
> the protocol "honest". Doesn't matter so much for this particular patch
> since don't think it actually touches the protocol.
>
> Ian.
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply
* [patch net] udp6: respect IPV6_DONTFRAG sockopt in case there are pending frames
From: Jiri Pirko @ 2013-09-30 13:50 UTC (permalink / raw)
To: netdev; +Cc: davem, kuznet, jmorris, hannes, kaber, yoshfuji
if up->pending != 0 dontfrag is left with default value -1. That
causes that application that do:
sendto len>mtu flag MSG_MORE
sendto len>mtu flag 0
will receive EMSGSIZE errno as the result of the second sendto.
This patch fixes it by respecting IPV6_DONTFRAG socket option.
introduced by:
commit 4b340ae20d0e2366792abe70f46629e576adaf5e "IPv6: Complete IPV6_DONTFRAG support"
Please push to stable as well.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/ipv6/udp.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 72b7eaa..1878609 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -1225,9 +1225,6 @@ do_udp_sendmsg:
if (tclass < 0)
tclass = np->tclass;
- if (dontfrag < 0)
- dontfrag = np->dontfrag;
-
if (msg->msg_flags&MSG_CONFIRM)
goto do_confirm;
back_from_confirm:
@@ -1246,6 +1243,8 @@ back_from_confirm:
up->pending = AF_INET6;
do_append_data:
+ if (dontfrag < 0)
+ dontfrag = np->dontfrag;
up->len += ulen;
getfrag = is_udplite ? udplite_getfrag : ip_generic_getfrag;
err = ip6_append_data(sk, getfrag, msg->msg_iov, ulen,
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net-next] xen-netfront: convert to GRO API
From: Ian Campbell @ 2013-09-30 13:24 UTC (permalink / raw)
To: Wei Liu; +Cc: netdev, xen-devel, konrad.wilk, Anirban Chakraborty
In-Reply-To: <1380545194-13419-1-git-send-email-wei.liu2@citrix.com>
On Mon, 2013-09-30 at 13:46 +0100, Wei Liu wrote:
> Anirban was seeing netfront received MTU size packets, which downgraded
> throughput. The following patch makes netfront use GRO API which
> improves throughput for that case.
>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> Cc: Anirban Chakraborty <abchak@juniper.net>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> Acked-by: Konrad Wilk <konrad.wilk@oracle.com>
> ---
> drivers/net/xen-netfront.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
> index 36808bf..dd1011e 100644
> --- a/drivers/net/xen-netfront.c
> +++ b/drivers/net/xen-netfront.c
> @@ -952,7 +952,7 @@ static int handle_incoming_queue(struct net_device *dev,
> u64_stats_update_end(&stats->syncp);
>
> /* Pass it up. */
> - netif_receive_skb(skb);
> + napi_gro_receive(&np->napi, skb);
> }
>
> return packets_dropped;
> @@ -1051,6 +1051,8 @@ err:
> if (work_done < budget) {
> int more_to_do = 0;
>
> + napi_gro_flush(napi, false);
> +
> local_irq_save(flags);
>
> RING_FINAL_CHECK_FOR_RESPONSES(&np->rx, more_to_do);
^ permalink raw reply
* Re: [PATCH 01/21] drivers: remove unnecessary prom.h includes
From: Marc Kleine-Budde @ 2013-09-30 13:23 UTC (permalink / raw)
To: Rob Herring
Cc: Rob Herring, linux-kernel, devicetree, Grant Likely,
Wolfgang Grandegger, linux-can, netdev
In-Reply-To: <52497A6E.5010908@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1096 bytes --]
On 09/30/2013 03:19 PM, Rob Herring wrote:
> On 09/30/2013 02:54 AM, Marc Kleine-Budde wrote:
>> On 09/26/2013 08:50 PM, Rob Herring wrote:
>>> From: Rob Herring <rob.herring@calxeda.com>
>>>
>>> Remove unnecessary prom.h includes in preparation to remove implicit
>>> includes of prom.h.
>>>
>>> Signed-off-by: Rob Herring <rob.herring@calxeda.com>
>>> Cc: Wolfgang Grandegger <wg@grandegger.com>
>>> Cc: Marc Kleine-Budde <mkl@pengutronix.de>
>>> Cc: linux-can@vger.kernel.org
>>> Cc: netdev@vger.kernel.org
>>
>> Applied to linux-can-next/testing, it will be included in my next pull
>> request to David Miller.
>
> Please don't apply this. The whole series needs to be applied together.
> This patch is a dependency for the rest of the series.
Okay, then add my Acked-by to this patch.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]
^ permalink raw reply
* Re: [PATCH 01/21] drivers: remove unnecessary prom.h includes
From: Rob Herring @ 2013-09-30 13:19 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: Rob Herring, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Grant Likely,
Wolfgang Grandegger, linux-can-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <52492E4F.2020701-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
On 09/30/2013 02:54 AM, Marc Kleine-Budde wrote:
> On 09/26/2013 08:50 PM, Rob Herring wrote:
>> From: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>>
>> Remove unnecessary prom.h includes in preparation to remove implicit
>> includes of prom.h.
>>
>> Signed-off-by: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
>> Cc: Wolfgang Grandegger <wg-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>
>> Cc: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
>> Cc: linux-can-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>
> Applied to linux-can-next/testing, it will be included in my next pull
> request to David Miller.
Please don't apply this. The whole series needs to be applied together.
This patch is a dependency for the rest of the series.
Rob
--
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
* [PATCH] ethernet: moxa: fix incorrect placement of __initdata tag
From: Bartlomiej Zolnierkiewicz @ 2013-09-30 13:18 UTC (permalink / raw)
To: David S. Miller; +Cc: Jonas Jensen, netdev, linux-kernel, Kyungmin Park
__initdata tag should be placed between the variable name and equal
sign for the variable to be placed in the intended .init.data section.
In this particular case __initdata is incorrect as moxart_mac_driver
can be used after the driver gets initialized.
Also while at it static-ize moxart_mac_driver.
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
drivers/net/ethernet/moxa/moxart_ether.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/moxa/moxart_ether.c b/drivers/net/ethernet/moxa/moxart_ether.c
index 83c2091..bd1a2d2 100644
--- a/drivers/net/ethernet/moxa/moxart_ether.c
+++ b/drivers/net/ethernet/moxa/moxart_ether.c
@@ -543,7 +543,7 @@ static const struct of_device_id moxart_mac_match[] = {
{ }
};
-struct __initdata platform_driver moxart_mac_driver = {
+static struct platform_driver moxart_mac_driver = {
.probe = moxart_mac_probe,
.remove = moxart_remove,
.driver = {
--
1.8.2.3
^ permalink raw reply related
* [PATCH net-next] xen-netfront: convert to GRO API
From: Wei Liu @ 2013-09-30 12:46 UTC (permalink / raw)
To: netdev; +Cc: xen-devel, konrad.wilk, Wei Liu, Anirban Chakraborty,
Ian Campbell
Anirban was seeing netfront received MTU size packets, which downgraded
throughput. The following patch makes netfront use GRO API which
improves throughput for that case.
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
Cc: Anirban Chakraborty <abchak@juniper.net>
Cc: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Konrad Wilk <konrad.wilk@oracle.com>
---
drivers/net/xen-netfront.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 36808bf..dd1011e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -952,7 +952,7 @@ static int handle_incoming_queue(struct net_device *dev,
u64_stats_update_end(&stats->syncp);
/* Pass it up. */
- netif_receive_skb(skb);
+ napi_gro_receive(&np->napi, skb);
}
return packets_dropped;
@@ -1051,6 +1051,8 @@ err:
if (work_done < budget) {
int more_to_do = 0;
+ napi_gro_flush(napi, false);
+
local_irq_save(flags);
RING_FINAL_CHECK_FOR_RESPONSES(&np->rx, more_to_do);
--
1.7.10.4
^ permalink raw reply related
* [PATCH net] skbuff: size of hole is wrong in a comment
From: Nicolas Dichtel @ 2013-09-30 12:16 UTC (permalink / raw)
To: davem; +Cc: netdev, mgorman, Nicolas Dichtel
Since commit c93bdd0e03e8 ("netvm: allow skb allocation to use PFMEMALLOC
reserves"), hole size is one bit less than what is written in the comment.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
include/linux/skbuff.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 2ddb48d9312c..c2d89335f637 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -498,7 +498,7 @@ struct sk_buff {
* headers if needed
*/
__u8 encapsulation:1;
- /* 7/9 bit hole (depending on ndisc_nodetype presence) */
+ /* 6/8 bit hole (depending on ndisc_nodetype presence) */
kmemcheck_bitfield_end(flags2);
#if defined CONFIG_NET_DMA || defined CONFIG_NET_RX_BUSY_POLL
--
1.8.2.1
^ permalink raw reply related
* [PATCH] can: flexcan: fix flexcan_chip_start() on imx6
From: Marc Kleine-Budde @ 2013-09-30 11:59 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel, Marc Kleine-Budde, linux-stable
In-Reply-To: <1380542343-5299-1-git-send-email-mkl@pengutronix.de>
In the flexcan_chip_start() function first the flexcan core is going through
the soft reset sequence, then the RX FIFO is enabled.
With the hardware is put into FIFO mode, message buffers 1...7 are reserved by
the FIFO engine. The remaining message buffers are in reset default values.
This patch removes the bogus initialization of the message buffers, as it
causes an imprecise external abort on imx6.
Cc: linux-stable <stable@vger.kernel.org>
Reported-by: Lothar Waßmann <LW@KARO-electronics.de>
Tested-by: Lothar Waßmann <LW@KARO-electronics.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
drivers/net/can/flexcan.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index 71c677e..3f21142 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -702,7 +702,6 @@ static int flexcan_chip_start(struct net_device *dev)
{
struct flexcan_priv *priv = netdev_priv(dev);
struct flexcan_regs __iomem *regs = priv->base;
- unsigned int i;
int err;
u32 reg_mcr, reg_ctrl;
@@ -772,17 +771,6 @@ static int flexcan_chip_start(struct net_device *dev)
netdev_dbg(dev, "%s: writing ctrl=0x%08x", __func__, reg_ctrl);
flexcan_write(reg_ctrl, ®s->ctrl);
- for (i = 0; i < ARRAY_SIZE(regs->cantxfg); i++) {
- flexcan_write(0, ®s->cantxfg[i].can_ctrl);
- flexcan_write(0, ®s->cantxfg[i].can_id);
- flexcan_write(0, ®s->cantxfg[i].data[0]);
- flexcan_write(0, ®s->cantxfg[i].data[1]);
-
- /* put MB into rx queue */
- flexcan_write(FLEXCAN_MB_CNT_CODE(0x4),
- ®s->cantxfg[i].can_ctrl);
- }
-
/* acceptance mask/acceptance code (accept everything) */
flexcan_write(0x0, ®s->rxgmask);
flexcan_write(0x0, ®s->rx14mask);
--
1.8.4.rc3
^ permalink raw reply related
* pull-request: can 2013-09-30
From: Marc Kleine-Budde @ 2013-09-30 11:59 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
here is a fix for the v3.12 release cycle. Lothar Waßmann reported a problem in
the flexcan driver that leads to an imprecise external abort on imx6. The patch
by me fixes the problem. It was tested by Lothar on imx53 and imx6.
regards,
Marc
---
The following changes since commit 9a3bab6b05383f1e4c3716b3615500c51285959e:
net: net_secret should not depend on TCP (2013-09-28 15:19:40 -0700)
are available in the git repository at:
git://gitorious.org/linux-can/linux-can.git fixes-for-3.12
for you to fetch changes up to 0d1862ea1a5bb876cf05555a7307080cb75bf379:
can: flexcan: fix flexcan_chip_start() on imx6 (2013-09-30 13:49:09 +0200)
----------------------------------------------------------------
Marc Kleine-Budde (1):
can: flexcan: fix flexcan_chip_start() on imx6
drivers/net/can/flexcan.c | 12 ------------
1 file changed, 12 deletions(-)
^ permalink raw reply
* Re: [PATCH 19/51] DMA-API: media: dt3155v4l: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Hans Verkuil @ 2013-09-30 11:57 UTC (permalink / raw)
To: Russell King
Cc: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
linux-wireless, netdev, Solarflare linux maintainers,
uclinux-dist-devel, Mauro Carvalho Chehab, Greg Kroah-Hartman
In-Reply-To: <E1VMm13-0007hO-9l@rmk-PC.arm.linux.org.uk>
On 09/19/2013 11:44 PM, Russell King wrote:
> Replace the following sequence:
>
> dma_set_mask(dev, mask);
> dma_set_coherent_mask(dev, mask);
>
> with a call to the new helper dma_set_mask_and_coherent().
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
Regards,
Hans
> ---
> drivers/staging/media/dt3155v4l/dt3155v4l.c | 5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/media/dt3155v4l/dt3155v4l.c b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> index 90d6ac4..081407b 100644
> --- a/drivers/staging/media/dt3155v4l/dt3155v4l.c
> +++ b/drivers/staging/media/dt3155v4l/dt3155v4l.c
> @@ -901,10 +901,7 @@ dt3155_probe(struct pci_dev *pdev, const struct pci_device_id *id)
> int err;
> struct dt3155_priv *pd;
>
> - err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> - if (err)
> - return -ENODEV;
> - err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> + err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> if (err)
> return -ENODEV;
> pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>
^ permalink raw reply
* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-30 11:46 UTC (permalink / raw)
To: vyasevic
Cc: Toshiaki Makita, David Miller, netdev, Fernando Luis Vazquez Cao,
Patrick McHardy
In-Reply-To: <5245C9FB.90307@redhat.com>
On Fri, 2013-09-27 at 14:10 -0400, Vlad Yasevich wrote:
> On 09/27/2013 01:11 PM, Toshiaki Makita wrote:
> > On Thu, 2013-09-26 at 10:22 -0400, Vlad Yasevich wrote:
> >> On 09/26/2013 06:38 AM, Toshiaki Makita wrote:
> >>> On Tue, 2013-09-24 at 13:55 -0400, Vlad Yasevich wrote:
> >>>> On 09/24/2013 01:30 PM, Toshiaki Makita wrote:
> >>>>> On Tue, 2013-09-24 at 09:35 -0400, Vlad Yasevich wrote:
> >>>>>> On 09/24/2013 07:45 AM, Toshiaki Makita wrote:
> >>>>>>> On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich wrote:
> >>>>>>>> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> >>>>>>>>> On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich wrote:
> >>>>>>>>>> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>>>>>>>>>> On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >>>>>>>>>>>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >>>>>>>>>>>> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>>>>>>>>>>>
> >>>>>>>>>>>>> There seem to be some undesirable behaviors related with PVID.
> >>>>>>>>>>>>> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >>>>>>>>>>>>> to any frame regardless of whether we set it or not.
> >>>>>>>>>>>>> 2. FDB entries learned via frames applied PVID are registered with
> >>>>>>>>>>>>> VID 0 rather than VID value of PVID.
> >>>>>>>>>>>>> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >>>>>>>>>>>>> This leads interoperational problems such as sending frames with VID
> >>>>>>>>>>>>> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >>>>>>>>>>>>> 0 as they belong to VLAN 0, which is expected to be handled as they have
> >>>>>>>>>>>>> no VID according to IEEE 802.1Q.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >>>>>>>>>>>>> is fixed, because we cannot activate PVID due to it.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Please work out the issues in patch #2 with Vlad and resubmit this
> >>>>>>>>>>>> series.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Thank you.
> >>>>>>>>>>>
> >>>>>>>>>>> I'm hovering between whether we should fix the issue by changing vlan 0
> >>>>>>>>>>> interface behavior in 8021q module or enabling a bridge port to sending
> >>>>>>>>>>> priority-tagged frames, or another better way.
> >>>>>>>>>>>
> >>>>>>>>>>> If you could comment it, I'd appreciate it :)
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> BTW, I think what is discussed in patch #2 is another problem about
> >>>>>>>>>>> handling priority-tags, and it exists without this patch set applied.
> >>>>>>>>>>> It looks like that we should prepare another patch set than this to fix
> >>>>>>>>>>> that problem.
> >>>>>>>>>>>
> >>>>>>>>>>> Should I include patches that fix the priority-tags problem in this
> >>>>>>>>>>> patch set and resubmit them all together?
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> I am thinking that we might need to do it in bridge and it looks like
> >>>>>>>>>> the simplest way to do it is to have default priority regeneration table
> >>>>>>>>>> (table 6-5 from 802.1Q doc).
> >>>>>>>>>>
> >>>>>>>>>> That way I think we would conform to the spec.
> >>>>>>>>>>
> >>>>>>>>>> -vlad
> >>>>>>>>>
> >>>>>>>>> Unfortunately I don't think the default priority regeneration table
> >>>>>>>>> resolves the problem because IEEE 802.1Q says that a VLAN-aware bridge
> >>>>>>>>> can transmit untagged or VLAN-tagged frames only (the end of section 7.5
> >>>>>>>>> and 8.1.7).
> >>>>>>>>>
> >>>>>>>>> No mechanism to send priority-tagged frames is found as far as I can see
> >>>>>>>>> the standard. I think the regenerated priority is used for outgoing PCP
> >>>>>>>>> field only if egress policy is not untagged (i.e. transmitting as
> >>>>>>>>> VLAN-tagged), and unused if untagged (Section 6.9.2 3rd/4th Paragraph).
> >>>>>>>>>
> >>>>>>>>> If we want to transmit priority-tagged frames from a bridge port, I
> >>>>>>>>> think we need to implement a new (optional) feature that is above the
> >>>>>>>>> standard, as I stated previously.
> >>>>>>>>>
> >>>>>>>>> How do you feel about adding a per-port policy that enables a bridge to
> >>>>>>>>> send priority-tagged frames instead of untagged frames when egress
> >>>>>>>>> policy for the port is untagged?
> >>>>>>>>> With this change, we can transmit frames for a given vlan as either all
> >>>>>>>>> untagged, all priority-tagged or all VLAN-tagged.
> >>>>>>>>
> >>>>>>>> That would work. What I am thinking is that we do it by special casing
> >>>>>>>> the vid 0 egress policy specification. Let it be untagged by default
> >>>>>>>> and if it is tagged, then we preserve the priority field and forward
> >>>>>>>> it on.
> >>>>>>>>
> >>>>>>>> This keeps the API stable and doesn't require user/admin from knowing
> >>>>>>>> exactly what happens. Default operation conforms to the spec and allows
> >>>>>>>> simple change to make it backward-compatible.
> >>>>>>>>
> >>>>>>>> What do you think. I've done a simple prototype of this an it seems to
> >>>>>>>> work with the VMs I am testing with.
> >>>>>>>
> >>>>>>> Are you saying that
> >>>>>>> - by default, set the 0th bit of untagged_bitmap; and
> >>>>>>> - if we unset the 0th bit and set the "vid"th bit, we transmit frames
> >>>>>>> classified as belonging to VLAN "vid" as priority-tagged?
> >>>>>>>
> >>>>>>> If so, though it's attractive to keep current API, I'm worried about if
> >>>>>>> it could be a bit confusing and not intuitive for kernel/iproute2
> >>>>>>> developers that VID 0 has a special meaning only in the egress policy.
> >>>>>>> Wouldn't it be better to adding a new member to struct net_port_vlans
> >>>>>>> instead of using VID 0 of untagged_bitmap?
> >>>>>>>
> >>>>>>> Or are you saying that we use a new flag in struct net_port_vlans but
> >>>>>>> use the BRIDGE_VLAN_INFO_UNTAGGED bit with VID 0 in netlink to set the
> >>>>>>> flag?
> >>>>>>>
> >>>>>>> Even in that case, I'm afraid that it might be confusing for developers
> >>>>>>> for the same reason. We are going to prohibit to specify VID with 0 (and
> >>>>>>> 4095) in adding/deleting a FDB entry or a vlan filtering entry, but it
> >>>>>>> would allow us to use VID 0 only when a vlan filtering entry is
> >>>>>>> configured.
> >>>>>>> I am thinking a new nlattr is a straightforward approach to configure
> >>>>>>> it.
> >>>>>>
> >>>>>> By making this an explicit attribute it makes vid 0 a special case for
> >>>>>> any automatic tool that would provision such filtering. Seeing vid 0
> >>>>>> would mean that these tools would have to know that this would have to
> >>>>>> be translated to a different attribute instead of setting the policy
> >>>>>> values.
> >>>>>
> >>>>> Yes, I agree with you that we can do it by the way you explained.
> >>>>> What I don't understand is the advantage of using vid 0 over another way
> >>>>> such as adding a new nlattr.
> >>>>> I think we can indicate transmitting priority-tags explicitly by such a
> >>>>> nlattr. Using vid 0 seems to be easier to implement than a new nlattr,
> >>>>> but, for me, it looks less intuitive and more difficult to maintain
> >>>>> because we have to care about vid 0 instead of simply ignoring it.
> >>>>>
> >>>>
> >>>> The point I am trying to make is that regardless of the approach someone
> >>>> has to know what to do when enabling priority tagged frames. You
> >>>> proposal would require the administrator or config tool to have that
> >>>> knowledge. Example is:
> >>>> Admin does: bridge vlan set priority on dev eth0
> >>>> Automated app:
> >>>> if (vid == 0)
> >>>> /* Turn on priority tagged frame support */
> >>>>
> >>>> My proposal would require the bridge filtering implementation to have it.
> >>>> user tool: bridge vlan add vid 0 tagged
> >>>> Automated app: No special case.
> >>>>
> >>>> IMO its better to have 1 piece code handling the special case then
> >>>> putting it multiple places.
> >>>
> >>> Thank you for the detailed explanation.
> >>> Now I understand your intention.
> >>>
> >>> I have one question about your proposal.
> >>> I guess the way to enable priority-tagged is something like
> >>> bridge vlan add vid 10 dev eth0
> >>> bridge vlan add vid 10 dev vnet0 pvid untagged
> >>> bridge vlan add vid 0 dev vnet0 tagged
> >>> where vnet0 has sub interface vnet0.0.
> >>>
> >>> Here the admin have to know the egress policy is applied to a frame
> >>> twice in a certain order when it is transmitted from the port vnet0
> >>> attached, that is, first, a frame with vid 10 get untagged, and then, an
> >>> untagged frame get priority-tagged.
> >>>
> >>> This behavior looks difficult to know without previous knowledge.
> >>> Any good idea to avoid such a need for the admin's additional knowledge?
> >>
> >> To me, the fact that there is vnet0.0 (or typically, there is eth0.0 in
> >> the guest or on the remote host) already tells the admin vlan 0 has to
> >> be tagged. The fact that we codify this in the policy makes it explicit.
> >
> > My worry is that the admin might not be able to guess how to use bridge
> > commands to enable priority-tag without any additional hint in "man
> > bridge", "bridge vlan help", etc.
> > I actually couldn't hit upon such a usage before seeing example commands
> > you gave, because I had never think the egress policy could be applied
> > twice.
> >
> >>
> >> However, I can see strong argument to be made for an addition egress
> >> policy attribute that could be for instance:
> >>
> >> bridge vlan add vid 10 dev eth0 pvid
> >> bridge vlan add vid 10 dev vnet0 pvid untagged prio_tag
> >>
> >> But this has the same connotations as wrt to egress policy. The 2
> >> policies are applied:
> >> (1) untag the frame.
> >> (2) add priority_tag.
> >>
> >> (2) only happens if initial fame received on eth0 was priority tagged.
> >
> > If we do so, we will not be able to communicate using vlan 0 interface
> > under a certain circumstance.
> > Eth0 can be receive mixed untagged and priority-tagged frames according
> > to the network element it is connected to: for example, Open vSwitch can
> > send such two kinds of frames from the same port even if original
> > incoming frames belong to the same vlan.
>
> Which priority would you assign to the frame that was received untagged?
Untagged frame's priority is by default 0, so I think 0 is likely.
802.1Q 6.9.1 i)
The received priority value and the drop_eligible parameter value are
the values in the M_UNITDATA.indication.
M_UNITDATA is passed from ISS.
802.1Q 6.7.1
The priority parameter provided in a data indication primitive shall
take the value of the Default User Priority parameter for the Port
through which the MAC frame was received. The default value of this
parameter is 0, it may be set by management in which case the
capability to set it to any of the values 0 through 7 shall be
provided.
>
> > In this situation, we can only receive frames that is priority-tagged
> > when received on eth0.
>
> Not sure I understand. Let's look at this config:
> bridge vlan add vid 10 dev eth0 pvid
> bridge vlan add vid 10 dev vnet0 pvid untagged prio_tag
>
> Here, eth0 is allowed to receive vid 10 tagged, untagged, and
> prio_tagged (if we look at the patch 2 from this set).
> Now, frame is forwarded to vnet0.
> 1) if the frame had vid 10 in the tag or was untagged,
> it should probably be sent untagged.
> 2) if the frame had a priority tag, it should probably
> be sent as such.
>
> Now, I think a case could be made that if the frame had any priority
> markings in the vlan header, we should try to preserve those markings
> if prio_tag is turned on. We can assume value of 0 means not set.
If we don't insert prio_tag when PCP is 0, we might receive mixed
priority-tagged and untagged frames on eth0.
Even if we are sending frames from eth0.0 with some priority other than
0, we could receive frames with priority 0 or untagged on the other side
of the bridge.
For example, if we receive untagged arp reply on the bridge port, we
migit not be able to communicate with such an end station, because
untagged reply will not be passed to eth0.0.
>
> > IMO, if prio_tag is configured, the bridge should send any untagged
> > frame as priority-tagged regardless of whatever it is on eth0.
>
> Which priority would you use, 0? You are not guaranteed to properly
> deliver the traffic then for a configuration such as:
> VM: eth0: 10.0.0.1/24
> eth0.0: 10.0.1.1/24
I'd like to use priority 0 for untagged frames.
I am assuming that one of our goals is at least that eth0.0 comes to be
able to communicate with another end station. It seems to be hard to use
both eth0 and eth0.0 simultaneously.
Thanks,
Toshiaki Makita
>
> -vlad
>
> >
> >>
> >> I think I am ok with either approach. Explicit vid 0 policy is easier
> >> for automatic provisioning. The flag based one is easier for admin/
> >> manual provisioning.
> >
> > Supposing we have to add something to help or man in any case, I think
> > flag based is better.
> > The format below seems to suitable for a per-port policy.
> > bridge vlan set prio_tag on dev vnet0
> >
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >> -vlad.
> >>
> >> -vlad
> >>
> >>
> >>
> >>
> >>>
> >>>>
> >>>> Thanks
> >>>> -vlad
> >>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Toshiaki Makita
> >>>>>
> >>>>>>
> >>>>>> How it is implemented internally in the kernel isn't as big of an issue.
> >>>>>> We can do it as a separate flag or as part of existing policy.
> >>>>>>
> >>>>>> -vlad
> >>>>>>
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>> Toshiaki Makita
> >>>>>>>
> >>>>>>>>
> >>>>>>>> -vlad
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>>
> >>>>>>>>> Toshiaki Makita
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks,
> >>>>>>>>>>>
> >>>>>>>>>>> Toshiaki Makita
> >>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>> --
> >>>>>>>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>>>>>>>>>> the body of a message to majordomo@vger.kernel.org
> >>>>>>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> --
> >>>>>>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>>>>>>> the body of a message to majordomo@vger.kernel.org
> >>>>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>
> >
> >
^ permalink raw reply
* Re: [PATCH] ipv6: udp packets following an UFO enqueued packet need also be handled by UFO
From: Jiri Pirko @ 2013-09-30 11:43 UTC (permalink / raw)
To: hannes; +Cc: netdev, yoshfuji, davem
In-Reply-To: <20130921042700.GB8070@order.stressinduktion.org>
Sat, Sep 21, 2013 at 06:27:00AM CEST, hannes@stressinduktion.org wrote:
>In the following scenario the socket is corked:
>If the first UDP packet is larger then the mtu we try to append it to the
>write queue via ip6_ufo_append_data. A following packet, which is smaller
>than the mtu would be appended to the already queued up gso-skb via
>plain ip6_append_data. This causes random memory corruptions.
>
>In ip6_ufo_append_data we also have to be careful to not queue up the
>same skb multiple times. So setup the gso frame only when no first skb
>is available.
>
>This also fixes a shortcoming where we add the current packet's length to
>cork->length but return early because of a packet > mtu with dontfrag set
>(instead of sutracting it again).
>
>Found with trinity.
>
>Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
>Signed-off-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
>---
>
>I could only test this with virtualized UFO enabled network cards. Could
>someone test this on real hardware?
>
> net/ipv6/ip6_output.c | 53 +++++++++++++++++++++------------------------------
> 1 file changed, 22 insertions(+), 31 deletions(-)
>
>diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
>index 3a692d5..a54c45c 100644
>--- a/net/ipv6/ip6_output.c
>+++ b/net/ipv6/ip6_output.c
>@@ -1015,6 +1015,8 @@ static inline int ip6_ufo_append_data(struct sock *sk,
> * udp datagram
> */
> if ((skb = skb_peek_tail(&sk->sk_write_queue)) == NULL) {
>+ struct frag_hdr fhdr;
>+
> skb = sock_alloc_send_skb(sk,
> hh_len + fragheaderlen + transhdrlen + 20,
> (flags & MSG_DONTWAIT), &err);
>@@ -1036,12 +1038,6 @@ static inline int ip6_ufo_append_data(struct sock *sk,
> skb->protocol = htons(ETH_P_IPV6);
> skb->ip_summed = CHECKSUM_PARTIAL;
> skb->csum = 0;
>- }
>-
>- err = skb_append_datato_frags(sk,skb, getfrag, from,
>- (length - transhdrlen));
>- if (!err) {
>- struct frag_hdr fhdr;
>
> /* Specify the length of each IPv6 datagram fragment.
> * It has to be a multiple of 8.
>@@ -1052,15 +1048,10 @@ static inline int ip6_ufo_append_data(struct sock *sk,
> ipv6_select_ident(&fhdr, rt);
> skb_shinfo(skb)->ip6_frag_id = fhdr.identification;
> __skb_queue_tail(&sk->sk_write_queue, skb);
>-
>- return 0;
> }
>- /* There is not enough support do UPD LSO,
>- * so follow normal path
>- */
>- kfree_skb(skb);
>
>- return err;
>+ return skb_append_datato_frags(sk, skb, getfrag, from,
>+ (length - transhdrlen));
> }
>
What if non-ufo-path-created skb is peeked?
^ permalink raw reply
* RE: Question regarding failure utilizing bonding mode 5 (balance-tlb)
From: Yuval Mintz @ 2013-09-30 11:30 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev@vger.kernel.org, Ariel Elior
In-Reply-To: <979A8436335E3744ADCD3A9F2A2B68A52ACF96BB@SJEXCHMB10.corp.ad.broadcom.com>
> > >Again, I think the permanent address is restored only when the bond
> > >releases the slave, which I don't think happens when the slave is unloaded.
> >
> > Ah, ok, I was understanding "unloaded" to mean "remove from the
> > bond." I think you actually mean "set administratively down," e.g., "ip
> > link set dev slave down" or the like. I don't think mere loss of
> > carrier would trigger the sequence of events, because that won't go
> > through a dev_close / dev_open cycle.
> >
> > Doing that (an admin down / up bounce) would, indeed, cause a
> > failover, but the bond will not reprogram the MAC on the slave (it
> > presumes that a fail / recovery will not disrupt the MAC address, which
> > is apparently not true in this instance).
> >
> > I'll have to look at the code a bit, but for now can you confirm
> > that what you actually mean is, essentially:
> >
> > Given a bond0 with two slaves, eth0 and eth1, in tlb mode, eth0
> > being the active,
> >
> > 1) "ip link set dev eth0 down" which will fail over to eth1
> > (swapping the contents of their dev_addr fields).
> >
> > 2) "ip link set dev eth0 up" eth0 comes back up, reprograms its
> > MAC to the wrong thing (what was in dev_addr).
> >
> > 3) repeat steps 1 and 2 for eth1
> >
> > Is this correct?
> >
>
> Yes, sorry for the earlier confusion.
> I think in the case described `alb_swap_mac_addr()' will be called,
> replacing eth0 and eth1's dev_addr, causing eth0 to have dev_addr
> which defers from the bond device's. Once eth0 reloads, it will use
> the different MAC address for configuring FW/HW.
Hi,
Did you by any chance had the time to look at this issue?
Thanks,
Yuval
^ permalink raw reply
* linux-next: manual merge of the net-next tree
From: Thierry Reding @ 2013-09-30 11:26 UTC (permalink / raw)
To: David S. Miller; +Cc: netdev, linux-next, linux-kernel
In-Reply-To: <1380540373-25352-1-git-send-email-treding@nvidia.com>
Today's linux-next merge of the net-next tree got conflicts in
arch/h8300/include/uapi/asm/socket.h
drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
include/net/secure_seq.h
I removed the h8300 file and I fixed up the other two (see below). Please
check if the resolution looks correct.
Thanks,
Thierry
---
diff --cc drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
index 74156f8,5bc0276..7f1340d
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
@@@ -132,35 -132,33 +132,34 @@@ struct pktq *brcmf_bus_gettxq(struct br
* interface functions from common layer
*/
- extern bool brcmf_c_prec_enq(struct device *dev, struct pktq *q,
- struct sk_buff *pkt, int prec);
+ bool brcmf_c_prec_enq(struct device *dev, struct pktq *q, struct sk_buff *pkt,
+ int prec);
/* Receive frame for delivery to OS. Callee disposes of rxp. */
- extern void brcmf_rx_frames(struct device *dev, struct sk_buff_head *rxlist);
+ void brcmf_rx_frames(struct device *dev, struct sk_buff_head *rxlist);
/* Indication from bus module regarding presence/insertion of dongle. */
- extern int brcmf_attach(uint bus_hdrlen, struct device *dev);
+ int brcmf_attach(uint bus_hdrlen, struct device *dev);
/* Indication from bus module regarding removal/absence of dongle */
- extern void brcmf_detach(struct device *dev);
+ void brcmf_detach(struct device *dev);
/* Indication from bus module that dongle should be reset */
- extern void brcmf_dev_reset(struct device *dev);
+ void brcmf_dev_reset(struct device *dev);
/* Indication from bus module to change flow-control state */
- extern void brcmf_txflowblock(struct device *dev, bool state);
+ void brcmf_txflowblock(struct device *dev, bool state);
/* Notify the bus has transferred the tx packet to firmware */
- extern void brcmf_txcomplete(struct device *dev, struct sk_buff *txp,
- bool success);
+ void brcmf_txcomplete(struct device *dev, struct sk_buff *txp, bool success);
- extern int brcmf_bus_start(struct device *dev);
+ int brcmf_bus_start(struct device *dev);
#ifdef CONFIG_BRCMFMAC_SDIO
- extern void brcmf_sdio_exit(void);
- extern void brcmf_sdio_init(void);
- extern void brcmf_sdio_register(void);
+ void brcmf_sdio_exit(void);
+ void brcmf_sdio_init(void);
++void brcmf_sdio_register(void);
#endif
#ifdef CONFIG_BRCMFMAC_USB
- extern void brcmf_usb_exit(void);
- extern void brcmf_usb_register(void);
+ void brcmf_usb_exit(void);
-void brcmf_usb_init(void);
++void brcmf_usb_register(void);
#endif
#endif /* _BRCMF_BUS_H_ */
diff --cc include/net/secure_seq.h
index c2e542b,52c1a90..f257486
--- a/include/net/secure_seq.h
+++ b/include/net/secure_seq.h
@@@ -3,18 -3,19 +3,18 @@@
#include <linux/types.h>
- extern __u32 secure_ip_id(__be32 daddr);
- extern __u32 secure_ipv6_id(const __be32 daddr[4]);
- extern u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
- extern u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
- __be16 dport);
- extern __u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
- __be16 sport, __be16 dport);
- extern __u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
- __be16 sport, __be16 dport);
- extern u64 secure_dccp_sequence_number(__be32 saddr, __be32 daddr,
- __be16 sport, __be16 dport);
- extern u64 secure_dccpv6_sequence_number(__be32 *saddr, __be32 *daddr,
- __be16 sport, __be16 dport);
-void net_secret_init(void);
+ __u32 secure_ip_id(__be32 daddr);
+ __u32 secure_ipv6_id(const __be32 daddr[4]);
+ u32 secure_ipv4_port_ephemeral(__be32 saddr, __be32 daddr, __be16 dport);
+ u32 secure_ipv6_port_ephemeral(const __be32 *saddr, const __be32 *daddr,
+ __be16 dport);
+ __u32 secure_tcp_sequence_number(__be32 saddr, __be32 daddr,
+ __be16 sport, __be16 dport);
+ __u32 secure_tcpv6_sequence_number(const __be32 *saddr, const __be32 *daddr,
+ __be16 sport, __be16 dport);
+ u64 secure_dccp_sequence_number(__be32 saddr, __be32 daddr,
+ __be16 sport, __be16 dport);
+ u64 secure_dccpv6_sequence_number(__be32 *saddr, __be32 *daddr,
+ __be16 sport, __be16 dport);
#endif /* _NET_SECURE_SEQ */
^ permalink raw reply
* linux-next: manual merge of the ipsec-next tree
From: Thierry Reding @ 2013-09-30 11:26 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu, David S. Miller
Cc: netdev, linux-next, linux-kernel
In-Reply-To: <1380540373-25352-1-git-send-email-treding@nvidia.com>
Today's linux-next merge of the ipsec-next tree got conflicts in
include/net/xfrm.h
I fixed it up (see below). Please check if the resolution looks correct.
Thanks,
Thierry
---
diff --cc include/net/xfrm.h
index 7657461,c7afa6e..041820c
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@@ -1493,39 -1495,35 +1499,39 @@@ static inline int xfrm4_rcv_spi(struct
return xfrm4_rcv_encap(skb, nexthdr, spi, 0);
}
-extern int xfrm4_extract_output(struct xfrm_state *x, struct sk_buff *skb);
-extern int xfrm4_prepare_output(struct xfrm_state *x, struct sk_buff *skb);
-extern int xfrm4_output(struct sk_buff *skb);
-extern int xfrm4_output_finish(struct sk_buff *skb);
-extern int xfrm4_tunnel_register(struct xfrm_tunnel *handler, unsigned short family);
-extern int xfrm4_tunnel_deregister(struct xfrm_tunnel *handler, unsigned short family);
-extern int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel_notifier *handler);
-extern int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel_notifier *handler);
-extern int xfrm6_extract_header(struct sk_buff *skb);
-extern int xfrm6_extract_input(struct xfrm_state *x, struct sk_buff *skb);
-extern int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi);
-extern int xfrm6_transport_finish(struct sk_buff *skb, int async);
-extern int xfrm6_rcv(struct sk_buff *skb);
-extern int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
- xfrm_address_t *saddr, u8 proto);
-extern int xfrm6_tunnel_register(struct xfrm6_tunnel *handler, unsigned short family);
-extern int xfrm6_tunnel_deregister(struct xfrm6_tunnel *handler, unsigned short family);
-extern __be32 xfrm6_tunnel_alloc_spi(struct net *net, xfrm_address_t *saddr);
-extern __be32 xfrm6_tunnel_spi_lookup(struct net *net, const xfrm_address_t *saddr);
-extern int xfrm6_extract_output(struct xfrm_state *x, struct sk_buff *skb);
-extern int xfrm6_prepare_output(struct xfrm_state *x, struct sk_buff *skb);
-extern int xfrm6_output(struct sk_buff *skb);
-extern int xfrm6_output_finish(struct sk_buff *skb);
-extern int xfrm6_find_1stfragopt(struct xfrm_state *x, struct sk_buff *skb,
- u8 **prevhdr);
+int xfrm4_extract_output(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm4_prepare_output(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm4_output(struct sk_buff *skb);
+int xfrm4_output_finish(struct sk_buff *skb);
+int xfrm4_tunnel_register(struct xfrm_tunnel *handler, unsigned short family);
+int xfrm4_tunnel_deregister(struct xfrm_tunnel *handler, unsigned short family);
- int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler);
- int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel *handler);
++int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel_notifier *handler);
++int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel_notifier *handler);
+void xfrm4_local_error(struct sk_buff *skb, u32 mtu);
+int xfrm6_extract_header(struct sk_buff *skb);
+int xfrm6_extract_input(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi);
+int xfrm6_transport_finish(struct sk_buff *skb, int async);
+int xfrm6_rcv(struct sk_buff *skb);
+int xfrm6_input_addr(struct sk_buff *skb, xfrm_address_t *daddr,
+ xfrm_address_t *saddr, u8 proto);
+int xfrm6_tunnel_register(struct xfrm6_tunnel *handler, unsigned short family);
+int xfrm6_tunnel_deregister(struct xfrm6_tunnel *handler,
+ unsigned short family);
+__be32 xfrm6_tunnel_alloc_spi(struct net *net, xfrm_address_t *saddr);
+__be32 xfrm6_tunnel_spi_lookup(struct net *net, const xfrm_address_t *saddr);
+int xfrm6_extract_output(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm6_prepare_output(struct xfrm_state *x, struct sk_buff *skb);
+int xfrm6_output(struct sk_buff *skb);
+int xfrm6_output_finish(struct sk_buff *skb);
+int xfrm6_find_1stfragopt(struct xfrm_state *x, struct sk_buff *skb,
+ u8 **prevhdr);
+void xfrm6_local_error(struct sk_buff *skb, u32 mtu);
#ifdef CONFIG_XFRM
-extern int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb);
-extern int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen);
+int xfrm4_udp_encap_rcv(struct sock *sk, struct sk_buff *skb);
+int xfrm_user_policy(struct sock *sk, int optname,
+ u8 __user *optval, int optlen);
#else
static inline int xfrm_user_policy(struct sock *sk, int optname, u8 __user *optval, int optlen)
{
^ permalink raw reply
* Re: [PATCH net-next 2/4] bonding: use RCU protection for alb xmit path
From: Ding Tianhong @ 2013-09-30 11:19 UTC (permalink / raw)
To: Veaceslav Falico
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <20130930105233.GA6096@redhat.com>
On 2013/9/30 18:52, Veaceslav Falico wrote:
> On Sun, Sep 29, 2013 at 07:45:05PM +0800, Ding Tianhong wrote:
>> The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb
>> (bonding: initial RCU conversion) has convert the roundrobin,
>> active-backup, broadcast and xor xmit path to rcu protection,
>> the performance will be better for these mode, so this time,
>> convert xmit path for alb mode.
>>
>> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>> Cc: Nikolay Aleksandrov <nikolay@redhat.com>
>> Cc: Veaceslav Falico <vfalico@redhat.com>
>> ---
>> drivers/net/bonding/bond_alb.c | 23 +++++++++--------------
>> drivers/net/bonding/bonding.h | 2 +-
>> 2 files changed, 10 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>> index e960418..a1444d5 100644
>> --- a/drivers/net/bonding/bond_alb.c
>> +++ b/drivers/net/bonding/bond_alb.c
>> @@ -230,7 +230,7 @@ static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
>> max_gap = LLONG_MIN;
>>
>> /* Find the slave with the largest gap */
>> - bond_for_each_slave(bond, slave, iter) {
>> + bond_for_each_slave_rcu(bond, slave, iter) {
>> if (SLAVE_IS_OK(slave)) {
>> long long gap = compute_gap(slave);
>>
>> @@ -387,7 +387,7 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
>> struct list_head *iter;
>> bool found = false;
>>
>> - bond_for_each_slave(bond, slave, iter) {
>> + bond_for_each_slave_rcu(bond, slave, iter) {
>> if (!SLAVE_IS_OK(slave))
>> continue;
>> if (!found) {
>> @@ -628,12 +628,14 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
>> {
>> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
>> struct arp_pkt *arp = arp_pkt(skb);
>> - struct slave *assigned_slave;
>> + struct slave *assigned_slave, *curr_active_slave;
>> struct rlb_client_info *client_info;
>> u32 hash_index = 0;
>>
>> _lock_rx_hashtbl(bond);
>>
>> + curr_active_slave = rcu_dereference(bond->curr_active_slave);
>> +
>> hash_index = _simple_hash((u8 *)&arp->ip_dst, sizeof(arp->ip_dst));
>> client_info = &(bond_info->rx_hashtbl[hash_index]);
>>
>> @@ -658,8 +660,8 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
>> * that the new client can be assigned to this entry.
>> */
>> if (bond->curr_active_slave &&
>> - client_info->slave != bond->curr_active_slave) {
>> - client_info->slave = bond->curr_active_slave;
>> + client_info->slave != curr_active_slave) {
>> + client_info->slave = curr_active_slave;
>> rlb_update_client(client_info);
>> }
>> }
>> @@ -1343,11 +1345,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> skb_reset_mac_header(skb);
>> eth_data = eth_hdr(skb);
>>
>> - /* make sure that the curr_active_slave do not change during tx
>> - */
>> - read_lock(&bond->lock);
>> - read_lock(&bond->curr_slave_lock);
>> -
>> switch (ntohs(skb->protocol)) {
>> case ETH_P_IP: {
>> const struct iphdr *iph = ip_hdr(skb);
>> @@ -1429,12 +1426,12 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>>
>> if (!tx_slave) {
>> /* unbalanced or unassigned, send through primary */
>> - tx_slave = bond->curr_active_slave;
>> + tx_slave = rcu_dereference(bond->curr_active_slave);
>> bond_info->unbalanced_load += skb->len;
>> }
>>
>> if (tx_slave && SLAVE_IS_OK(tx_slave)) {
>> - if (tx_slave != bond->curr_active_slave) {
>> + if (tx_slave != rcu_dereference(bond->curr_active_slave)) {
>> memcpy(eth_data->h_source,
>> tx_slave->dev->dev_addr,
>> ETH_ALEN);
>> @@ -1449,8 +1446,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>> }
>> }
>>
>> - read_unlock(&bond->curr_slave_lock);
>> - read_unlock(&bond->lock);
>> if (res) {
>> /* no suitable interface, frame not sent */
>> kfree_skb(skb);
>> diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>> index 713b6af..1c7d559 100644
>> --- a/drivers/net/bonding/bonding.h
>> +++ b/drivers/net/bonding/bonding.h
>> @@ -459,7 +459,7 @@ static inline struct slave *bond_slave_has_mac(struct bonding *bond,
>> struct list_head *iter;
>> struct slave *tmp;
>>
>> - bond_for_each_slave(bond, tmp, iter)
>> + bond_for_each_slave_rcu(bond, tmp, iter)
>
> This suggests that rcu_read_lock() is always held here, however it's not
> the case, as far as I can tell:
>
> bond_release()/bond_uninit() ->
> __bond_release_one() ->
> bond_alb_deinit_slave() ->
> alb_change_hw_addr_on_detach() ->
> bond_slave_has_mac()
>
> There is nobody that takes rcu_read_lock() there. And, for example, when
> doing:
>
> echo -$slave > /sys/class/net/$bond_int/bonding/slaves
>
> nobody is holding it. And, btw, in bond_for_each_slave_rcu(), which is
> actually a wrapper for netdev_for_each_lower_private_rcu(), which calls
> netdev_lower_get_next_private_rcu(), which has:
>
> 4543 void *netdev_lower_get_next_private_rcu(struct net_device *dev,
> 4544 struct list_head **iter)
> 4545 {
> 4546 struct netdev_adjacent *lower;
> 4547 4548 WARN_ON_ONCE(!rcu_read_lock_held());
>
> so you should have seen that warning when trying to remove a slave in alb
> mode.
>
>> if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
>> return tmp;
>>
>> --
>> 1.8.2.1
>>
>>
>>
oh, yes, has to do little more,thanks.
>
> .
>
^ permalink raw reply
* Re: [PATCH net-next 2/4] bonding: use RCU protection for alb xmit path
From: Veaceslav Falico @ 2013-09-30 10:52 UTC (permalink / raw)
To: Ding Tianhong
Cc: Jay Vosburgh, Andy Gospodarek, David S. Miller,
Nikolay Aleksandrov, Netdev
In-Reply-To: <524812C1.6070407@huawei.com>
On Sun, Sep 29, 2013 at 07:45:05PM +0800, Ding Tianhong wrote:
>The commit 278b20837511776dc9d5f6ee1c7fabd5479838bb
>(bonding: initial RCU conversion) has convert the roundrobin,
>active-backup, broadcast and xor xmit path to rcu protection,
>the performance will be better for these mode, so this time,
>convert xmit path for alb mode.
>
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>Signed-off-by: Yang Yingliang <yangyingliang@huawei.com>
>Cc: Nikolay Aleksandrov <nikolay@redhat.com>
>Cc: Veaceslav Falico <vfalico@redhat.com>
>---
> drivers/net/bonding/bond_alb.c | 23 +++++++++--------------
> drivers/net/bonding/bonding.h | 2 +-
> 2 files changed, 10 insertions(+), 15 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c
>index e960418..a1444d5 100644
>--- a/drivers/net/bonding/bond_alb.c
>+++ b/drivers/net/bonding/bond_alb.c
>@@ -230,7 +230,7 @@ static struct slave *tlb_get_least_loaded_slave(struct bonding *bond)
> max_gap = LLONG_MIN;
>
> /* Find the slave with the largest gap */
>- bond_for_each_slave(bond, slave, iter) {
>+ bond_for_each_slave_rcu(bond, slave, iter) {
> if (SLAVE_IS_OK(slave)) {
> long long gap = compute_gap(slave);
>
>@@ -387,7 +387,7 @@ static struct slave *rlb_next_rx_slave(struct bonding *bond)
> struct list_head *iter;
> bool found = false;
>
>- bond_for_each_slave(bond, slave, iter) {
>+ bond_for_each_slave_rcu(bond, slave, iter) {
> if (!SLAVE_IS_OK(slave))
> continue;
> if (!found) {
>@@ -628,12 +628,14 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
> {
> struct alb_bond_info *bond_info = &(BOND_ALB_INFO(bond));
> struct arp_pkt *arp = arp_pkt(skb);
>- struct slave *assigned_slave;
>+ struct slave *assigned_slave, *curr_active_slave;
> struct rlb_client_info *client_info;
> u32 hash_index = 0;
>
> _lock_rx_hashtbl(bond);
>
>+ curr_active_slave = rcu_dereference(bond->curr_active_slave);
>+
> hash_index = _simple_hash((u8 *)&arp->ip_dst, sizeof(arp->ip_dst));
> client_info = &(bond_info->rx_hashtbl[hash_index]);
>
>@@ -658,8 +660,8 @@ static struct slave *rlb_choose_channel(struct sk_buff *skb, struct bonding *bon
> * that the new client can be assigned to this entry.
> */
> if (bond->curr_active_slave &&
>- client_info->slave != bond->curr_active_slave) {
>- client_info->slave = bond->curr_active_slave;
>+ client_info->slave != curr_active_slave) {
>+ client_info->slave = curr_active_slave;
> rlb_update_client(client_info);
> }
> }
>@@ -1343,11 +1345,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> skb_reset_mac_header(skb);
> eth_data = eth_hdr(skb);
>
>- /* make sure that the curr_active_slave do not change during tx
>- */
>- read_lock(&bond->lock);
>- read_lock(&bond->curr_slave_lock);
>-
> switch (ntohs(skb->protocol)) {
> case ETH_P_IP: {
> const struct iphdr *iph = ip_hdr(skb);
>@@ -1429,12 +1426,12 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
>
> if (!tx_slave) {
> /* unbalanced or unassigned, send through primary */
>- tx_slave = bond->curr_active_slave;
>+ tx_slave = rcu_dereference(bond->curr_active_slave);
> bond_info->unbalanced_load += skb->len;
> }
>
> if (tx_slave && SLAVE_IS_OK(tx_slave)) {
>- if (tx_slave != bond->curr_active_slave) {
>+ if (tx_slave != rcu_dereference(bond->curr_active_slave)) {
> memcpy(eth_data->h_source,
> tx_slave->dev->dev_addr,
> ETH_ALEN);
>@@ -1449,8 +1446,6 @@ int bond_alb_xmit(struct sk_buff *skb, struct net_device *bond_dev)
> }
> }
>
>- read_unlock(&bond->curr_slave_lock);
>- read_unlock(&bond->lock);
> if (res) {
> /* no suitable interface, frame not sent */
> kfree_skb(skb);
>diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
>index 713b6af..1c7d559 100644
>--- a/drivers/net/bonding/bonding.h
>+++ b/drivers/net/bonding/bonding.h
>@@ -459,7 +459,7 @@ static inline struct slave *bond_slave_has_mac(struct bonding *bond,
> struct list_head *iter;
> struct slave *tmp;
>
>- bond_for_each_slave(bond, tmp, iter)
>+ bond_for_each_slave_rcu(bond, tmp, iter)
This suggests that rcu_read_lock() is always held here, however it's not
the case, as far as I can tell:
bond_release()/bond_uninit() ->
__bond_release_one() ->
bond_alb_deinit_slave() ->
alb_change_hw_addr_on_detach() ->
bond_slave_has_mac()
There is nobody that takes rcu_read_lock() there. And, for example, when
doing:
echo -$slave > /sys/class/net/$bond_int/bonding/slaves
nobody is holding it. And, btw, in bond_for_each_slave_rcu(), which is
actually a wrapper for netdev_for_each_lower_private_rcu(), which calls
netdev_lower_get_next_private_rcu(), which has:
4543 void *netdev_lower_get_next_private_rcu(struct net_device *dev,
4544 struct list_head **iter)
4545 {
4546 struct netdev_adjacent *lower;
4547
4548 WARN_ON_ONCE(!rcu_read_lock_held());
so you should have seen that warning when trying to remove a slave in alb
mode.
> if (ether_addr_equal_64bits(mac, tmp->dev->dev_addr))
> return tmp;
>
>--
>1.8.2.1
>
>
>
^ permalink raw reply
* RE: [PATCH] {iproute2, xfrm}: Use memcpy to suppress gcc phony buffer overflow warning
From: David Laight @ 2013-09-30 9:29 UTC (permalink / raw)
To: Sohny Thomas, Fan Du; +Cc: stephen, netdev
In-Reply-To: <52479CA0.4050505@linux.vnet.ibm.com>
> > This is a false positive warning as the destination pointer "buf"
> > pointers to
> > an ZERO length array, which actually will occupy alg.buf mostly.
> > Fix this by using memcpy.
> >
> > struct xfrm_algo {
> > char alg_name[64];
> > unsigned int alg_key_len; /* in bits */
> > char alg_key[0];
> > };
> >
> > struct {
> > union {
> > struct xfrm_algo alg;
> > struct xfrm_algo_aead aead;
> > struct xfrm_algo_auth auth;
> > } u;
> > char buf[XFRM_ALGO_KEY_BUF_SIZE];
> > } alg = {};
> >
> > buf = alg.u.alg.alg_key;
That is worse than horrid...
The tools have every right to complain about any accesses to alg_key[].
> > ---
> > ip/xfrm_state.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
> > index 0d98e78..5cc87d3 100644
> > --- a/ip/xfrm_state.c
> > +++ b/ip/xfrm_state.c
> > @@ -159,7 +159,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg,
> > enum xfrm_attr_type_t type,
> > if (len > max)
> > invarg("\"ALGO-KEY\" makes buffer overflow\n", key);
I presume there is a return hiding in invarg().
> >
> > - strncpy(buf, key, len);
> > + memcpy(buf, key, len);
Passing the length of the SOURCE to strncpy() is almost always wrong.
You are still not terminating the copied string.
David
^ permalink raw reply
* RE: [PATCH v1 net-next] net: pkt_sched: PIE AQM scheme
From: David Laight @ 2013-09-30 9:19 UTC (permalink / raw)
To: Vijay Subramanian, netdev
Cc: davem, shemminger, eric.dumazet, Mythili Prabhu, Dave Taht
In-Reply-To: <1380333383-9507-1-git-send-email-subramanian.vijay@gmail.com>
> +#define PIE_DEFAULT_QUEUE_LIMIT 200 /* in packets */
> +#define QUEUE_THRESHOLD (5000)
> +#define DQCOUNT_INVALID -1
> +#define THRESHOLD_PKT_SIZE 1500
> +#define MAX_INT_VALUE 0xffffffff
> +#define MAX_INT_VALUE_CAP (0xffffffff >> 8)
The above 2 constants are both unsigned (unless 'int' is larger than 32 bits).
If nothing else this means that they are very badly named.
> +
> +typedef u32 pie_time_t;
> +typedef s32 pie_tdiff_t;
> +#define PIE_SHIFT 10
> +#define MS2PIETIME(a) ((a * NSEC_PER_MSEC) >> PIE_SHIFT)
> +#define PIE_TIME_PER_SEC ((NSEC_PER_SEC >> PIE_SHIFT))
> +
> +static inline pie_time_t pie_get_time(void)
> +{
> + u64 ns = ktime_to_ns(ktime_get());
> + return ns >> PIE_SHIFT;
> +}
> +
> +static inline u32 pie_time_to_ms(pie_time_t val)
> +{
> + u64 valms = ((u64) val << PIE_SHIFT);
> +
> + do_div(valms, NSEC_PER_MSEC);
> + return (u32) valms;
> +}
There seems to be a lot of conversions between 1/1000 ms and 1/1024 ms.
On 32 bit systems they are horrid.
Not only that the conversions are open coded a lot of times as well.
It might also be better replacing '(val * 1000u) >> 10' with
'(val * (u64)(1000 << (32 - 10))) >> 32' since I'm not sure the compiler
will perform that substitution.
What happens if pie_time_to_ms() overflows 32 bits?
If it is only ever called for small values, there may be no need for
64bit maths at all.
...
> + if (delta > (s32) (MAX_INT_VALUE * 2 / 100)
> + && q->vars.prob >= MAX_INT_VALUE / 10) {
> + delta = MAX_INT_VALUE * 2 / 100;
Calculating 'MAX_INT_VALUE * 2' isn't a good idea!
Whatever value it is supposed to be, it should be a named constant.
David
^ permalink raw reply
* Re: [PATCH net-next] xen-netfront: convert to GRO API and advertise this feature
From: Ian Campbell @ 2013-09-30 9:12 UTC (permalink / raw)
To: David Miller; +Cc: wei.liu2, netdev, xen-devel, abchak
In-Reply-To: <20130928.153800.327243541797748645.davem@davemloft.net>
On Sat, 2013-09-28 at 15:38 -0400, David Miller wrote:
> From: Wei Liu <wei.liu2@citrix.com>
> Date: Sat, 21 Sep 2013 17:05:43 +0100
>
> > @@ -1371,7 +1373,8 @@ static struct net_device *xennet_create_dev(struct xenbus_device *dev)
> > netif_napi_add(netdev, &np->napi, xennet_poll, 64);
> > netdev->features = NETIF_F_IP_CSUM | NETIF_F_RXCSUM |
> > NETIF_F_GSO_ROBUST;
> > - netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO;
> > + netdev->hw_features = NETIF_F_IP_CSUM | NETIF_F_SG | NETIF_F_TSO |
> > + NETIF_F_GRO;
>
> Please post a new version of this patch with the feedback you've been
> given integrated, in particular with this part removed because it is
> not necessary.
>
> Ian, please review the patch when Wei posts it.
I will, but note:
$ ./scripts/get_maintainer.pl -f drivers/net/xen-netfront.c
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> (supporter:XEN HYPERVISOR IN...)
Jeremy Fitzhardinge <jeremy@goop.org> (supporter:XEN HYPERVISOR IN...)
xen-devel@lists.xensource.com (moderated list:XEN HYPERVISOR IN...)
virtualization@lists.linux-foundation.org (open list:XEN HYPERVISOR IN...)
netdev@vger.kernel.org (open list:NETWORKING DRIVERS)
linux-kernel@vger.kernel.org (open list)
Strictly speaking I maintain netback not front, so Wei please remember
to CC the right people (mainly Konrad) as well as me.
BTW I think this separation is a good thing since it keeps changes to
the protocol "honest". Doesn't matter so much for this particular patch
since don't think it actually touches the protocol.
Ian.
^ permalink raw reply
* Re: [PATCH V5 net-next 2/3] net: huawei_cdc_ncm: Introduce the huawei_cdc_ncm driver
From: Oliver Neukum @ 2013-09-30 9:07 UTC (permalink / raw)
To: Enrico Mioso
Cc: Greg Kroah-Hartman, David S. Miller, Steve Glendinning,
Robert de Vries, Hayes Wang, Freddy Xin, Bjørn Mork,
Liu Junliang, open list, open list:USB NETWORKING DR...,
open list:NETWORKING DRIVERS, ModemManager-devel
In-Reply-To: <1380516609-31242-3-git-send-email-mrkiko.rs@gmail.com>
On Mon, 2013-09-30 at 04:50 +0000, Enrico Mioso wrote:
> +static int huawei_cdc_ncm_manage_power(struct usbnet *usbnet_dev, int on)
> +{
> + struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> + int rv = 0;
> +
> + if ((on && atomic_add_return(1, &drvstate->pmcount) == 1) ||
> + (!on && atomic_dec_and_test(&drvstate->pmcount))) {
> + rv = usb_autopm_get_interface(usbnet_dev->intf);
> + if (rv < 0)
> + goto err;
The error case corrupts drvstate->pmcount
> + usbnet_dev->intf->needs_remote_wakeup = on;
> + usb_autopm_put_interface(usbnet_dev->intf);
> + }
> +err:
> + return rv;
> +}
> +static int huawei_cdc_ncm_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> + int ret = 0;
> + struct usbnet *usbnet_dev = usb_get_intfdata(intf);
> + struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> + struct cdc_ncm_ctx *ctx = drvstate->ctx;
> +
> + if (ctx == NULL) {
> + ret = -1;
That is not a valid way to indicate an error.
> + goto error;
> + }
> +
> + ret = usbnet_suspend(intf, message);
> + if (ret < 0)
> + goto error;
> +
> + if (intf == ctx->control &&
> + drvstate->subdriver &&
> + drvstate->subdriver->suspend)
> + ret = drvstate->subdriver->suspend(intf, message);
> + if (ret < 0)
> + usbnet_resume(intf);
> +
> +error:
> + return ret;
> +}
> +
> +static int huawei_cdc_ncm_resume(struct usb_interface *intf)
> +{
> + int ret = 0;
> + struct usbnet *usbnet_dev = usb_get_intfdata(intf);
> + struct huawei_cdc_ncm_state *drvstate = (void *)&usbnet_dev->data;
> + bool callsub;
> + struct cdc_ncm_ctx *ctx = drvstate->ctx;
> +
> + /* should we call subdriver's resume function? */
> + callsub =
> + (intf == ctx->control &&
> + drvstate->subdriver &&
> + drvstate->subdriver->resume);
> +
> + if (callsub)
> + ret = drvstate->subdriver->resume(intf);
> + if (ret < 0)
> + goto err;
> + ret = usbnet_resume(intf);
> + if (ret < 0 && callsub && drvstate->subdriver->suspend)
You really want drivers with a resume() but no suspend() method?
> + drvstate->subdriver->suspend(intf, PMSG_SUSPEND);
> +err:
> + return ret;
> +}
Regards
Oliver
^ 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