* Re: mvneta: oops in __rcu_read_lock on mirabox
From: Willy Tarreau @ 2013-09-18 16:49 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Ethan Tuttle, Russell King - ARM Linux, Andrew Lunn, Jason Cooper,
netdev, Ezequiel Garcia, Gregory Clément, linux-arm-kernel
In-Reply-To: <20130918183549.3e7b8f4c@skate>
On Wed, Sep 18, 2013 at 06:35:49PM +0200, Thomas Petazzoni wrote:
> Dear Ethan Tuttle,
>
> On Tue, 17 Sep 2013 23:30:56 -0700, Ethan Tuttle wrote:
> > On Mon, Sep 16, 2013 at 11:01 PM, Willy Tarreau <w@1wt.eu> wrote:
> > > Next step should be that you test both kernels to be sure.
> >
> > Thanks for the kernel images, Willy. I'm still experimenting but
> > initial results are strange: I haven't seen a crash from the -ethan
> > image you provided, nor by a kernel with that config that I built
> > myself. The config is only different from my crashing config by a few
> > options. So perhaps some combination of options prevents the crash.
> > I'll see if I can narrow it down.
>
> A toolchain generating some crappy code maybe? Ethan, Willy, comparing
> your toolchain (compiler version, origin of the toolchain) could be
> interesting.
I thought about this but it looks suspicious, I don't see why the toolchain
would produce random bitflips.
My toolchain is a linaro 4.7 gcc into which I have added support for a
"pj4b" CPU target which is essentially the same as cortex-a9 plus support
for the IDIV instruction in thumb mode.
But I can send it to Ethan if that helps.
Willy
^ permalink raw reply
* Re: [PATCH] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Aida Mynzhasova @ 2013-09-18 16:40 UTC (permalink / raw)
To: Richard Cochran; +Cc: netdev
In-Reply-To: <20130918160846.GA4397@netboy>
On 18.09.2013 20:08, Richard Cochran wrote:
> On Wed, Sep 18, 2013 at 05:21:04PM +0400, Aida Mynzhasova wrote:
>> IEEE 1588 timer reference clock source is determined through hard-coded
>> value in gianfar_ptp driver by default. This patch allows to select ptp
>> clock source by means of device tree file node.
>>
>> For instance:
>>
>> fsl,cksel = <0>;
>>
>> for using external (TSEC_TMR_CLK input) high precision timer
>> reference clock.
>>
>> Other acceptable values:
>>
>> <1> : eTSEC system clock
>> <2> : eTSEC1 transmit clock
>> <3> : RTC clock input
>
> Do the other clock sources even work at all?
>
> We were not able to get them working.
>
> Thanks,
> Richard
>
Hi Richard,
I've tried to use 2 clock sources: external from TSEC_TMR_CLK input (for
this I had to update multiplexer settings in uboot) and eTSEC system
clock - counter worked fine with both.
My attempts to use eTSEC1 transmit clock and RTC clock were unsuccessful
(the system hanged up) irrespective of using hard-coded value or entry
in dts file.
Thanks.
^ permalink raw reply
* Re: [PATCH] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Aida Mynzhasova @ 2013-09-18 16:39 UTC (permalink / raw)
To: Claudiu Manoil; +Cc: richardcochran, netdev
In-Reply-To: <5239CA4F.9000600@freescale.com>
On 18.09.2013 19:44, Claudiu Manoil wrote:
>
> On 9/18/2013 4:21 PM, Aida Mynzhasova wrote:
>> IEEE 1588 timer reference clock source is determined through hard-coded
>> value in gianfar_ptp driver by default. This patch allows to select ptp
>> clock source by means of device tree file node.
>>
>> For instance:
>>
>> fsl,cksel = <0>;
>>
>
> Has this device tree binding been defined? (Where?)
> I don't see this property in the net-next.git tree at least.
>
> Claudiu
>
>
Hi Claudiu,
actually, I don't know where I should define this binding, my only idea
is to add "cksel" property description in
/Documentation/devicetree/bindings/net/fsl-tsec-phy.txt. Am I right or I
need to do some additional changes?
Thanks.
^ permalink raw reply
* Re: [PATCH] net: tsi108: Prevent compiler warning
From: David Miller @ 2013-09-18 16:38 UTC (permalink / raw)
To: thierry.reding; +Cc: netdev, linux-kernel
In-Reply-To: <1379508595-30252-1-git-send-email-treding@nvidia.com>
From: Thierry Reding <thierry.reding@gmail.com>
Date: Wed, 18 Sep 2013 14:49:55 +0200
> The dump_eth_one() function is only used if DEBUG is enabled, so protect
> it by a corresponding #ifdef DEBUG block.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
I would prefer that this function and the one call site is simply
removed instead.
This kind of thing can be provided via ethtool register dumps
and therefore without all of the ugliness this ad-hoc stuff
presents.
^ permalink raw reply
* Re: [PATCH ] ipv6: handle the update of the NDISC_REDIRECT error code in icmpv6_err_convert
From: David Miller @ 2013-09-18 16:36 UTC (permalink / raw)
To: duanj.fnst; +Cc: netdev, hannes
In-Reply-To: <523996E0.9080702@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Wed, 18 Sep 2013 20:04:48 +0800
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> when dealing with redirect message in udpv6_err() and
> rawv6_err() the err shoud be assigned to 0, not EPROTO.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
No, you should fix this the same way you handled DCCP, but skipping
the whole socket error setting path for NDISC_REDIRECT.
Clearing the socket error to zero is not correct, it means you will
lose any existing error setting.
^ permalink raw reply
* Re: mvneta: oops in __rcu_read_lock on mirabox
From: Thomas Petazzoni @ 2013-09-18 16:35 UTC (permalink / raw)
To: Ethan Tuttle
Cc: Willy Tarreau, Russell King - ARM Linux, Andrew Lunn,
Jason Cooper, netdev, Ezequiel Garcia, Gregory Clément,
linux-arm-kernel
In-Reply-To: <CACzLR4t2ABZdRk40VUhLtDKXX2PQiSVoQZQ4coQkNkvETnY6Rw@mail.gmail.com>
Dear Ethan Tuttle,
On Tue, 17 Sep 2013 23:30:56 -0700, Ethan Tuttle wrote:
> On Mon, Sep 16, 2013 at 11:01 PM, Willy Tarreau <w@1wt.eu> wrote:
> > Next step should be that you test both kernels to be sure.
>
> Thanks for the kernel images, Willy. I'm still experimenting but
> initial results are strange: I haven't seen a crash from the -ethan
> image you provided, nor by a kernel with that config that I built
> myself. The config is only different from my crashing config by a few
> options. So perhaps some combination of options prevents the crash.
> I'll see if I can narrow it down.
A toolchain generating some crappy code maybe? Ethan, Willy, comparing
your toolchain (compiler version, origin of the toolchain) could be
interesting.
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH ] net:dccp: do not report ICMP redirects to user space
From: David Miller @ 2013-09-18 16:34 UTC (permalink / raw)
To: duanj.fnst; +Cc: netdev, hannes
In-Reply-To: <5239968F.1070402@cn.fujitsu.com>
From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Date: Wed, 18 Sep 2013 20:03:27 +0800
> From: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> DCCP shouldn't be setting sk_err on redirects as it
> isn't an error condition. it should be doing exactly
> what tcp is doing and leaving the error handler without
> touching the socket.
>
> Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
Applied.
^ permalink raw reply
* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: Joe Perches @ 2013-09-18 16:25 UTC (permalink / raw)
To: David Miller; +Cc: nikolay, netdev
In-Reply-To: <20130918.121539.1213515779326200577.davem@davemloft.net>
On Wed, 2013-09-18 at 12:15 -0400, David Miller wrote:
> From: Nikolay Aleksandrov <nikolay@redhat.com>
> Date: Tue, 17 Sep 2013 16:12:35 +0200
>
> > @@ -1284,15 +1284,15 @@ EXPORT_SYMBOL_GPL(__netpoll_free_async);
> >
> > void netpoll_cleanup(struct netpoll *np)
> > {
> > - if (!np->dev)
> > - return;
> > -
> > rtnl_lock();
> > + if (!np->dev) {
> > + rtnl_unlock();
> > + return;
> > + }
> > __netpoll_cleanup(np);
> > - rtnl_unlock();
> > -
> > dev_put(np->dev);
> > np->dev = NULL;
> > + rtnl_unlock();
> > }
> > EXPORT_SYMBOL(netpoll_cleanup);
>
> I know it seems arbitrary, but please do this like:
>
> lock();
> if (condition) {
> ...
> }
> unlock();
>
> rather than having multiple return/unlock code paths.
>
> This style I am suggesting is easier to audit for locking problems.
Another alternative is:
lock();
if (!condition)
goto out;
...
out:
unlock();
return ...;
}
^ permalink raw reply
* Re: [PATCH net-next v4] Don't destroy the netdev until the vif is shut down
From: Ian Campbell @ 2013-09-18 16:25 UTC (permalink / raw)
To: Paul Durrant
Cc: Wei Liu, xen-devel@lists.xen.org, netdev@vger.kernel.org,
David Vrabel
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD010D0DB@AMSPEX01CL01.citrite.net>
On Wed, 2013-09-18 at 17:04 +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Ian Campbell [mailto:ian.campbell@citrix.com]
> > Sent: 18 September 2013 16:58
> > To: Wei Liu
> > Cc: Paul Durrant; xen-devel@lists.xen.org; netdev@vger.kernel.org; David
> > Vrabel
> > Subject: Re: [PATCH net-next v4] Don't destroy the netdev until the vif is
> > shut down
> >
> > On Wed, 2013-09-18 at 11:37 +0100, Wei Liu wrote:
> > > On Tue, Sep 17, 2013 at 05:46:08PM +0100, Paul Durrant wrote:
> > > > Without this patch, if a frontend cycles through states Closing
> > > > and Closed (which Windows frontends need to do) then the netdev
> > > > will be destroyed and requires re-invocation of hotplug scripts
> > > > to restore state before the frontend can move to Connected. Thus
> > > > when udev is not in use the backend gets stuck in InitWait.
> > > >
> > > > With this patch, the netdev is left alone whilst the backend is
> > > > still online and is only de-registered and freed just prior to
> > > > destroying the vif (which is also nicely symmetrical with the
> > > > netdev allocation and registration being done during probe) so
> > > > no re-invocation of hotplug scripts is required.
> > > >
> > > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > > Cc: David Vrabel <david.vrabel@citrix.com>
> > > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > >
> > > Acked-by: Wei Liu <wei.liu2@citrix.com>
> >
> > yeah, looks good, thanks.
> >
> > Paul, did you test this with non-Windows frontends too? and do things
> > like vif hot(un)plug still work?
> >
>
> Ian,
>
> I tested with a debian (wheezy) PV guest but I didn't test unplug. I
> cycled a windows frontend several times (which is how I spotted the
> tx_irq thing), and shutdown and brought up both the debian and windows
> guests several times. I can test unplug too if you'd like.
I don't think it needs to be a blocker for accepting this patch but it
would be good to try it, it's the sort of area which historically gets
broken by this sort of change.
Ian.
>
> Paul
^ permalink raw reply
* Re: [PATCH net 0/2] Fix cnic regressions
From: David Miller @ 2013-09-18 16:24 UTC (permalink / raw)
To: mchan; +Cc: netdev
In-Reply-To: <1379494239-5481-1-git-send-email-mchan@broadcom.com>
From: "Michael Chan" <mchan@broadcom.com>
Date: Wed, 18 Sep 2013 01:50:37 -0700
> Fix regressions caused by Doorbell change and macro change.
Both applied, thanks.
^ permalink raw reply
* Re: pull request net: batman-adv 2013-09-18
From: David Miller @ 2013-09-18 16:23 UTC (permalink / raw)
To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1379485658-2317-1-git-send-email-ordex@autistici.org>
From: Antonio Quartulli <ordex@autistici.org>
Date: Wed, 18 Sep 2013 08:27:37 +0200
> this is a very small (but important) fix intended for net/linux-3.{11,12} (since
> 3.12-rc1 is out I guess this patch needs to be enqueued for stable in order to
> reach 3.11?)
>
> This change fixes a regression introduced in 3.11 that prevents the Bridge Loop
> Avoidance component from correctly operate on VLANs.
Pulled and queued up for -stable.
^ permalink raw reply
* Re: [PATCH Resend 1/3] net: cdc-phonet: Staticize usbpn_probe
From: David Miller @ 2013-09-18 16:21 UTC (permalink / raw)
To: sachin.kamat; +Cc: netdev, remi
In-Reply-To: <1379475001-24505-1-git-send-email-sachin.kamat@linaro.org>
From: Sachin Kamat <sachin.kamat@linaro.org>
Date: Wed, 18 Sep 2013 08:59:59 +0530
> @@ -328,7 +328,8 @@ MODULE_DEVICE_TABLE(usb, usbpn_ids);
>
> static struct usb_driver usbpn_driver;
>
> -int usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
> +static int
> +usbpn_probe(struct usb_interface *intf, const struct usb_device_id *id)
Please keep it to one line, it's 82 columns which is fine.
^ permalink raw reply
* Re: [PATCH net] ip: generate unique IP identificator if local fragmentation is allowed
From: David Miller @ 2013-09-18 16:20 UTC (permalink / raw)
To: aatteka; +Cc: netdev
In-Reply-To: <1379456700-8033-1-git-send-email-aatteka@nicira.com>
From: Ansis Atteka <aatteka@nicira.com>
Date: Tue, 17 Sep 2013 15:25:00 -0700
> @@ -1317,6 +1317,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
> ttl = ip_select_ttl(inet, &rt->dst);
>
> iph = (struct iphdr *)skb->data;
> +
> iph->version = 4;
> iph->ihl = 5;
> iph->tos = inet->tos;
> @@ -1324,7 +1325,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
> iph->ttl = ttl;
> iph->protocol = sk->sk_protocol;
> ip_copy_addrs(iph, fl4);
> - ip_select_ident(iph, &rt->dst, sk);
> + ip_select_ident(skb, &rt->dst, sk);
>
> if (opt) {
> iph->ihl += opt->optlen>>2;
This transformation is not equivalent.
ip_select_ident() is going to use ip_hdr() which obtains the header
pointer via skb_network_header().
But here, the code is using skb->data for this, which is different.
^ permalink raw reply
* Re: [PATCH -net] netpoll: fix NULL pointer dereference in netpoll_cleanup
From: David Miller @ 2013-09-18 16:15 UTC (permalink / raw)
To: nikolay; +Cc: netdev
In-Reply-To: <1379427155-8561-1-git-send-email-nikolay@redhat.com>
From: Nikolay Aleksandrov <nikolay@redhat.com>
Date: Tue, 17 Sep 2013 16:12:35 +0200
> @@ -1284,15 +1284,15 @@ EXPORT_SYMBOL_GPL(__netpoll_free_async);
>
> void netpoll_cleanup(struct netpoll *np)
> {
> - if (!np->dev)
> - return;
> -
> rtnl_lock();
> + if (!np->dev) {
> + rtnl_unlock();
> + return;
> + }
> __netpoll_cleanup(np);
> - rtnl_unlock();
> -
> dev_put(np->dev);
> np->dev = NULL;
> + rtnl_unlock();
> }
> EXPORT_SYMBOL(netpoll_cleanup);
I know it seems arbitrary, but please do this like:
lock();
if (condition) {
...
}
unlock();
rather than having multiple return/unlock code paths.
This style I am suggesting is easier to audit for locking problems.
Thanks.
^ permalink raw reply
* Re: [PATCH] USBNET: fix handling padding packet
From: David Miller @ 2013-09-18 16:12 UTC (permalink / raw)
To: bjorn; +Cc: ming.lei, gregkh, oneukum, netdev, linux-usb, oliver
In-Reply-To: <87zjra5dpx.fsf@nemi.mork.no>
From: Bjørn Mork <bjorn@mork.no>
Date: Wed, 18 Sep 2013 17:52:42 +0200
> Ming Lei <ming.lei@canonical.com> writes:
>
>> There is no reason to forbid DMA SG for one driver which requires
>> padding, right?
>
> Yes there is: Added complexity for everybody, based on a combination of
> features which just does not make any sense.
>
> No modern device should need the padding. No old device will be able to
> use the SG feature as implemented. You only enable it on USB3, don't
> you? If this feature is restricted to USB3 capable devices, then it most
> certainly can be restricted to ZLP capable devices with absolutely no
> difference in the resulting set of supported devices.
I completely agree.
^ permalink raw reply
* Re: [RFC PATCHv2 3/4] of: provide a binding for fixed link PHYs
From: Thomas Petazzoni @ 2013-09-18 16:11 UTC (permalink / raw)
To: Grant Likely
Cc: David S. Miller, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Lior Amsalem, Mark Rutland,
Sascha Hauer, Christian Gmeiner, Ezequiel Garcia, Gregory Clement,
Florian Fainelli,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20130918042923.5D845C42CF7-WNowdnHR2B42iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
Dear Grant Likely,
On Tue, 17 Sep 2013 23:29:23 -0500, Grant Likely wrote:
> I understand what you're trying to do here, but it causes a troublesome
> leakage of implementation detail into the binding, making the whole
> thing look very odd. This binding tries to make a fixed link look
> exactly like a real PHY even to the point of including a phandle to the
> phy. But having a phandle to a node which is *always* a direct child of
> the MAC node is redundant and a rather looney. Yes, doing it that way
> makes it easy for of_phy_find_device() to be transparent for fixed link,
> but that should *not* drive bindings, especially when that makes the
> binding really rather weird.
>
> Second, this new binding doesn't provide anything over and above the
> existing fixed-link binding. It may not be pretty, but it is
> estabilshed.
Have you followed the past discussions about this patch set? Basically
the *only* feedback I received on RFCv1 is that the fixed-link property
sucks, and everybody (including the known Device Tree binding
maintainer Mark Rutland) suggested to not use the fixed-link mechanism.
See http://article.gmane.org/gmane.linux.network/276932, where Mark
said:
""
I'm not sure grouping these values together is the best way of handling
this. It's rather opaque, and inflexible for future extension.
""
So, please DT maintainers, tell me what you want. I honestly don't care
whether fixed-link or a separate node is chosen. However, I do care
about being dragged around between two solutions just because the
former DT maintainer and the new DT maintainers do not agree.
Thanks,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Richard Cochran @ 2013-09-18 16:08 UTC (permalink / raw)
To: Aida Mynzhasova; +Cc: netdev
In-Reply-To: <1379510464-25420-1-git-send-email-aida.mynzhasova@skitlab.ru>
On Wed, Sep 18, 2013 at 05:21:04PM +0400, Aida Mynzhasova wrote:
> IEEE 1588 timer reference clock source is determined through hard-coded
> value in gianfar_ptp driver by default. This patch allows to select ptp
> clock source by means of device tree file node.
>
> For instance:
>
> fsl,cksel = <0>;
>
> for using external (TSEC_TMR_CLK input) high precision timer
> reference clock.
>
> Other acceptable values:
>
> <1> : eTSEC system clock
> <2> : eTSEC1 transmit clock
> <3> : RTC clock input
Do the other clock sources even work at all?
We were not able to get them working.
Thanks,
Richard
^ permalink raw reply
* RE: [PATCH net-next v4] Don't destroy the netdev until the vif is shut down
From: Paul Durrant @ 2013-09-18 16:04 UTC (permalink / raw)
To: Ian Campbell, Wei Liu
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, David Vrabel
In-Reply-To: <1379519895.11304.268.camel@hastur.hellion.org.uk>
> -----Original Message-----
> From: Ian Campbell [mailto:ian.campbell@citrix.com]
> Sent: 18 September 2013 16:58
> To: Wei Liu
> Cc: Paul Durrant; xen-devel@lists.xen.org; netdev@vger.kernel.org; David
> Vrabel
> Subject: Re: [PATCH net-next v4] Don't destroy the netdev until the vif is
> shut down
>
> On Wed, 2013-09-18 at 11:37 +0100, Wei Liu wrote:
> > On Tue, Sep 17, 2013 at 05:46:08PM +0100, Paul Durrant wrote:
> > > Without this patch, if a frontend cycles through states Closing
> > > and Closed (which Windows frontends need to do) then the netdev
> > > will be destroyed and requires re-invocation of hotplug scripts
> > > to restore state before the frontend can move to Connected. Thus
> > > when udev is not in use the backend gets stuck in InitWait.
> > >
> > > With this patch, the netdev is left alone whilst the backend is
> > > still online and is only de-registered and freed just prior to
> > > destroying the vif (which is also nicely symmetrical with the
> > > netdev allocation and registration being done during probe) so
> > > no re-invocation of hotplug scripts is required.
> > >
> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > Cc: David Vrabel <david.vrabel@citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> >
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
>
> yeah, looks good, thanks.
>
> Paul, did you test this with non-Windows frontends too? and do things
> like vif hot(un)plug still work?
>
Ian,
I tested with a debian (wheezy) PV guest but I didn't test unplug. I cycled a windows frontend several times (which is how I spotted the tx_irq thing), and shutdown and brought up both the debian and windows guests several times. I can test unplug too if you'd like.
Paul
^ permalink raw reply
* Re: [PATCH net-next v4] Don't destroy the netdev until the vif is shut down
From: Ian Campbell @ 2013-09-18 15:58 UTC (permalink / raw)
To: Wei Liu; +Cc: Paul Durrant, xen-devel, netdev, David Vrabel
In-Reply-To: <20130918103704.GA511@zion.uk.xensource.com>
On Wed, 2013-09-18 at 11:37 +0100, Wei Liu wrote:
> On Tue, Sep 17, 2013 at 05:46:08PM +0100, Paul Durrant wrote:
> > Without this patch, if a frontend cycles through states Closing
> > and Closed (which Windows frontends need to do) then the netdev
> > will be destroyed and requires re-invocation of hotplug scripts
> > to restore state before the frontend can move to Connected. Thus
> > when udev is not in use the backend gets stuck in InitWait.
> >
> > With this patch, the netdev is left alone whilst the backend is
> > still online and is only de-registered and freed just prior to
> > destroying the vif (which is also nicely symmetrical with the
> > netdev allocation and registration being done during probe) so
> > no re-invocation of hotplug scripts is required.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
yeah, looks good, thanks.
Paul, did you test this with non-Windows frontends too? and do things
like vif hot(un)plug still work?
Ian.
^ permalink raw reply
* Re: [PATCH 007/007] WAN Drivers: Update farsync driver and introduce fsflex driver
From: Ben Hutchings @ 2013-09-18 15:57 UTC (permalink / raw)
To: Kevin Curtis
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org, Dermot Smith
In-Reply-To: <E603DC592C92B54A89CEF6B0919A0B1CAAAA787DA6@SOLO.hq.farsitecommunications.com>
On Wed, 2013-09-18 at 11:12 +0100, Kevin Curtis wrote:
> Farsite Communications FarSync driver update
>
> Patch 7 of 7
>
> Update the help text and description for farsync configuration in the Kernel.
> Build farsync and fsflex when the farsync driver is selected.
fsflex seems to be an entirely independent module and should have its
own config option.
Ben.
> Signed-off-by: Kevin Curtis <kevin.curtis@farsite.com>
>
> ---
>
> diff -uprN -X linux-3.10.1/Documentation/dontdiff linux-3.10.1/drivers/net/wan/Kconfig linux-3.10.1_new/drivers/net/wan/Kconfig
> --- linux-3.10.1/drivers/net/wan/Kconfig 2013-07-13 19:42:41.000000000 +0100
> +++ linux-3.10.1_new/drivers/net/wan/Kconfig 2013-07-31 14:11:05.792775800 +0100
> @@ -248,11 +248,11 @@ config C101
> If unsure, say N.
>
> config FARSYNC
> - tristate "FarSync T-Series support"
> + tristate "FarSync T-Series and Flex support"
> depends on HDLC && PCI
> ---help---
> - Support for the FarSync T-Series X.21 (and V.35/V.24) cards by
> - FarSite Communications Ltd.
> + Support for the FarSync T-Series and FarSync Flex X.21 (and
> + V.35/V.24) ports by FarSite Communications Ltd.
>
> Synchronous communication is supported on all ports at speeds up to
> 8Mb/s (128K on V.24) using synchronous PPP, Cisco HDLC, raw HDLC,
> diff -uprN -X linux-3.10.1/Documentation/dontdiff linux-3.10.1/drivers/net/wan/Makefile linux-3.10.1_new/drivers/net/wan/Makefile
> --- linux-3.10.1/drivers/net/wan/Makefile 2013-07-13 19:42:41.000000000 +0100
> +++ linux-3.10.1_new/drivers/net/wan/Makefile 2013-07-26 09:14:15.345354002 +0100
> @@ -16,7 +16,7 @@ obj-$(CONFIG_HDLC_X25) += hdlc_x25.o
> obj-$(CONFIG_HOSTESS_SV11) += z85230.o hostess_sv11.o
> obj-$(CONFIG_SEALEVEL_4021) += z85230.o sealevel.o
> obj-$(CONFIG_COSA) += cosa.o
> -obj-$(CONFIG_FARSYNC) += farsync.o
> +obj-$(CONFIG_FARSYNC) += farsync.o fsflex.o
> obj-$(CONFIG_DSCC4) += dscc4.o
> obj-$(CONFIG_X25_ASY) += x25_asy.o
>
> --
> 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
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH 006/007] WAN Drivers: Update farsync driver and introduce fsflex driver
From: Ben Hutchings @ 2013-09-18 15:56 UTC (permalink / raw)
To: Kevin Curtis
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org, Dermot Smith
In-Reply-To: <E603DC592C92B54A89CEF6B0919A0B1CAAAA787DA5@SOLO.hq.farsitecommunications.com>
On Wed, 2013-09-18 at 11:12 +0100, Kevin Curtis wrote:
> Farsite Communications FarSync driver update
>
> Patch 6 of 7
>
> Introduce the fsflex driver.
> This driver is functionally equivalent to the farsync driver, and so
> can be used with the Generic HDLC, and ppp daemon.
>
> Signed-off-by: Kevin Curtis <kevin.curtis@farsite.com>
>
> ---
> diff -uprN -X linux-3.10.1/Documentation/dontdiff linux-3.10.1/drivers/net/wan/fsflex.c linux-3.10.1_new/drivers/net/wan/fsflex.c
> --- linux-3.10.1/drivers/net/wan/fsflex.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-3.10.1_new/drivers/net/wan/fsflex.c 2013-09-16 16:30:06.651104873 +0100
> @@ -0,0 +1,5693 @@
> +/*
> + * FarSync Flex driver for Linux (2.6.x kernel version)
Not exactly...
[...]
> +#define FST_MAX_CARDS 32
I don't think it's acceptable to set a static limit on number of devices
in a new driver.
[...]
> +int fst_txq_low = FST_LOW_WATER_MARK;
> +int fst_txq_high = FST_HIGH_WATER_MARK;
> +int fst_min_dma_len = FST_MIN_DMA_LEN;
> +int fst_dmathr = 0xdd00dd00;
> +int fst_iocinfo_version = FST_VERSION_CURRENT;
> +int fst_max_reads = 7;
> +int fst_excluded_cards;
> +int fst_excluded_list[FST_MAX_CARDS];
> +int fst_num_serials;
> +char *fst_serials[FST_MAX_CARDS];
All of which should be declared static, not implicitly extern. As
should all static variables and functions in this source file, as it's a
self-contained driver.
> +module_param(fst_txq_low, int, S_IRUGO);
> +module_param(fst_txq_high, int, S_IRUGO);
> +module_param(fst_min_dma_len, int, S_IRUGO);
> +module_param(fst_dmathr, int, S_IRUGO);
> +module_param(fst_iocinfo_version, int, S_IRUGO);
> +module_param(fst_max_reads, int, S_IRUGO);
> +module_param(fst_excluded_cards, int, S_IRUGO);
> +module_param_array(fst_excluded_list, int, NULL, S_IRUGO);
> +module_param_array(fst_serials, charp, &fst_num_serials, S_IRUGO);
Device configuration should be exposed through sysfs or ioctls, not
through module parameters.
[...]
> +#ifdef ARPHRD_RAWHDLC
> +#define ARPHRD_MYTYPE ARPHRD_RAWHDLC /* Raw frames */
> +#else
> +#define ARPHRD_MYTYPE ARPHRD_HDLC /* Cisco-HDLC (keepalives etc) */
> +#endif
#ifdef is not necessary in-tree.
[...]
> +#define usb_buffer_alloc usb_alloc_coherent
> +#define usb_buffer_free usb_free_coherent
Don't make up your own names for USB functions.
[...]
> +static int __init fst_init(void)
> +{
[...]
> + /* Create the /proc entry for /proc/fsflex
> + */
> + proc_create("fsflex", 0, NULL, &farsync_proc_fops);
[...]
Drivers should not be adding to the top level of /proc, and generally
should be adding any magic files under sysfs (per-device) or debugfs.
There is a /proc/driver directory if this information really doesn't fit
into either of those filesystems.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] USBNET: fix handling padding packet
From: Bjørn Mork @ 2013-09-18 15:52 UTC (permalink / raw)
To: Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum,
Network Development, linux-usb, Oliver Neukum
In-Reply-To: <CACVXFVNzTGbCV2wQ4RwxXK1q7+eD2KkOS_G3NvL4uAzdd21SUg@mail.gmail.com>
Ming Lei <ming.lei@canonical.com> writes:
>> Are you really sure that the one driver/device using this really need
>> the padding byte? If you could just make FLAG_SEND_ZLP part of the
>> condition for enabling can_dma_sg, then all this extra complexity would
>> be unnecessary. As the comment in front of the padding code says:
>
> At least for the effected driver of ax88179, the padding packet is needed
> since the driver does set the padding flag in the header, see
> ax88179_tx_fixup().
Yes, I noticed that the driver doesn't set the flag. I just didn't put
too much into that. I don't think that necessarily means that the
padding is needed. It probably just means that the driver worked with
the default padding enabled, so the author left it there.
This flag should really have been inverted. ZLP should be the default
for all new usbnet drivers.
Why don't you test it on the device you tested the SG patch with? I am
pretty sure it works just fine using proper ZLP transfer termination.
>> "strictly conforming cdc-ether devices should expect the ZLP here"
>>
>> There shouldn't be any problems requiring this conformance as a
>> precondition for allowing SG. The requirements are already strict.
>
> There is no reason to forbid DMA SG for one driver which requires
> padding, right?
Yes there is: Added complexity for everybody, based on a combination of
features which just does not make any sense.
No modern device should need the padding. No old device will be able to
use the SG feature as implemented. You only enable it on USB3, don't
you? If this feature is restricted to USB3 capable devices, then it most
certainly can be restricted to ZLP capable devices with absolutely no
difference in the resulting set of supported devices.
Anyway, if you want to keep the padding for SG then maybe this will work
and allow you to drop the extra struct usbnet field and allocation:
if (skb_tailroom(skb) && !dev->can_dma_sg) {
skb->data[skb->len] = 0;
__skb_put(skb, 1);
} else if (dev->can_dma_sg) {
sg_set_buf(&urb->sg[urb->num_sgs++], skb->data, 1);
}
I.e. cheat and use the skb->data buffer twice, if that is allowed? The
actual value of the padding byte should not matter, I believe?
Bjørn
^ permalink raw reply
* RE: [PATCH] USBNET: fix handling padding packet
From: David Laight @ 2013-09-18 15:43 UTC (permalink / raw)
To: Bjørn Mork, Ming Lei
Cc: David S. Miller, Greg Kroah-Hartman, Oliver Neukum, netdev,
linux-usb, Oliver Neukum
In-Reply-To: <878uyu6xjm.fsf@nemi.mork.no>
> I also believe it would be nice to move the initialisation of can_dma_sg
> away from the minidriver and into usbnet_probe. It's confusing that
> this field is used "uninitialized" (well, defaulting to zero) in all but
> one minidriver. It would much nicer if the logic was more like
>
> usbnet_probe:
> if (...)
> dev->can_dma_sg = 1;
>
> minidriver_bind:
> if (dev->can_dma_sg) {
> dev->net->features |= NETIF_F_SG | NETIF_F_TSO;
> dev->net->hw_features |= NETIF_F_SG | NETIF_F_TSO;
> }
Actually it would probably be nicer if the minidriver set
a flag to indicate that it could support fragmented skb
(a lot can't because of the way they add trailers)
and also provided the length of the header it will add.
The usbnet code could then allocate the header space.
If scatter-gather dma is available (a host feature) then
the header can be allocated outside the skb data area
to avoid having to copy the entire skb data.
The check for ZLP avoidance could then be done once only
(not sure how the info would be passed to teh minidriver apart
from adding more additional parameters to teh tx_fixup function).
I did a quick scan of the minidrivers, some of them don't
seem to have the correct checks for fragmented skb (etc).
Most of them only add a header.
David
^ permalink raw reply
* Re: [PATCH 005/007] WAN Drivers: Update farsync driver and introduce fsflex driver
From: Ben Hutchings @ 2013-09-18 15:44 UTC (permalink / raw)
To: Kevin Curtis
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org, Dermot Smith
In-Reply-To: <E603DC592C92B54A89CEF6B0919A0B1CAAAA787DA4@SOLO.hq.farsitecommunications.com>
On Wed, 2013-09-18 at 11:12 +0100, Kevin Curtis wrote:
> Farsite Communications FarSync driver update
>
> Patch 5 of 7
> Note that this patch must be applied with patch 4 (farsync_include_patch)
>
> Update the current farsync driver to support all of the PCI and PCI X
> cards manufactured by Farsite Communications.
>
> Add a tty interface so that the ports can also be used by the ppp daemon.
>
> Add support for big endian systems (the FarSite cards have a little endian
> processor on board).
[...]
Should all be separate patches. This is just doing way too much in one
go.
And there are regressions in here like:
> static struct pci_driver fst_driver = {
> - .name = FST_NAME,
> - .id_table = fst_pci_dev_id,
> - .probe = fst_add_one,
> - .remove = fst_remove_one,
> - .suspend = NULL,
> - .resume = NULL,
> +name: FST_NAME,
> +id_table : fst_pci_dev_id,
> +probe : fst_add_one,
> +remove : fst_remove_one,
> +suspend : NULL,
> +resume : NULL,
> +};
And the addition of a file in procfs (at the top level, even!) as if
it's still 2001 and sysfs doesn't exist:
> +static int farsync_proc_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, fst_proc_info, NULL);
> +}
> +
> +static const struct file_operations farsync_proc_fops = {
> + .open = farsync_proc_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> };
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH] powerpc/83xx: gianfar_ptp: select 1588 clock source through dts file
From: Claudiu Manoil @ 2013-09-18 15:44 UTC (permalink / raw)
To: Aida Mynzhasova; +Cc: richardcochran, netdev
In-Reply-To: <1379510464-25420-1-git-send-email-aida.mynzhasova@skitlab.ru>
On 9/18/2013 4:21 PM, Aida Mynzhasova wrote:
> IEEE 1588 timer reference clock source is determined through hard-coded
> value in gianfar_ptp driver by default. This patch allows to select ptp
> clock source by means of device tree file node.
>
> For instance:
>
> fsl,cksel = <0>;
>
Has this device tree binding been defined? (Where?)
I don't see this property in the net-next.git tree at least.
Claudiu
^ 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