* Re: FW: [PATCH V1 net-next 1/2] pgtable: Add API to query if write combining is available
From: Moshe Lazer @ 2014-10-08 8:44 UTC (permalink / raw)
To: davem
Cc: Or Gerlitz, Jack Morgenstein, Tal Alon, Yevgeny Petrilin, netdev,
Amir Vadai
In-Reply-To: <925ad10b2ec44e228e69bf0cbe6c0a0e@AMSPR05MB002.eurprd05.prod.outlook.com>
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Tuesday, October 07, 2014 10:44 PM
> To: Or Gerlitz
> Cc: netdev@vger.kernel.org; Amir Vadai; jackm@dev.mellanox.co.il; Moshe Lazer; Tal Alon; Yevgeny Petrilin
> Subject: Re: [PATCH V1 net-next 1/2] pgtable: Add API to query if write combining is available
>
> From: Or Gerlitz <ogerlitz@mellanox.com>
> Date: Sun, 5 Oct 2014 11:22:21 +0300
>
>> From: Moshe Lazer <moshel@mellanox.com>
>>
>> Currently the kernel write-combining interface provides a best effort
>> mechanism in which the caller simply invokes pgprot_writecombine().
>>
>> If write combining is available, the region is mapped for it,
>> otherwise the region is (silently) mapped as non-cached.
>>
>> In some cases, however, the calling driver must know if write
>> combining is available, so a silent best effort mechanism is not sufficient.
>>
>> Add writecombine_available(), which returns true if the system
>> supports write combining and false if it doesn't.
>>
>> Signed-off-by: Moshe Lazer <moshel@mellanox.com>
>> Signed-off-by: Jack Morgenstein <jackm@dev.mellanox.co.il>
>> Signed-off-by: Or Gerlitz <ogerlitz@mellanox.com>
> This needs some ACKs from MM developers.
>
> But also the situation is more complicated than a simple boolean test.
>
> On some platforms you have to test first whether the range you are trying to write combine can legally be marked in that way. The DRM layer has all of these per-arch tests to do this properly.
>
> #if defined(__i386__) || defined(__x86_64__)
> if (map->type == _DRM_REGISTERS && !(map->flags & _DRM_WRITE_COMBINING))
> tmp = pgprot_noncached(tmp);
> else
> tmp = pgprot_writecombine(tmp);
> #elif defined(__powerpc__)
> pgprot_val(tmp) |= _PAGE_NO_CACHE;
> if (map->type == _DRM_REGISTERS)
> pgprot_val(tmp) |= _PAGE_GUARDED;
> #elif defined(__ia64__)
> if (efi_range_is_wc(vma->vm_start, vma->vm_end -
> vma->vm_start))
> tmp = pgprot_writecombine(tmp);
> else
> tmp = pgprot_noncached(tmp);
> #elif defined(__sparc__) || defined(__arm__) || defined(__mips__)
> tmp = pgprot_noncached(tmp);
> #endif
The idea was to provide an indication as for whether the arch supports
write-combining in general.
If we want to benefit from blue flame operations, we need to map the
blue flame registers as write combining - otherwise there is no benefit.
So we would like to know if write combining is supported by the system
or not.
^ permalink raw reply
* RE: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding doc
From: luwei.zhou @ 2014-10-08 8:36 UTC (permalink / raw)
To: Richard Cochran
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Fabio.Estevam@freescale.com,
fugang.duan@freescale.com, Frank.Li@freescale.com,
stephen@networkplumber.org
In-Reply-To: <20141008080603.GA4648@netboy>
On Wed, Oct 8, 2014 at 4:06:00AM, Richard Cochran wrote:
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Wednesday, October 08, 2014 4:06 PM
> To: Zhou Luwei-B45643
> Cc: davem@davemloft.net; netdev@vger.kernel.org; shawn.guo@linaro.org;
> bhutchings@solarflare.com; Estevam Fabio-R49496; Duan Fugang-B38611; Li
> Frank-B20596; stephen@networkplumber.org
> Subject: Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding
> doc
>
> On Wed, Oct 08, 2014 at 03:15:08AM +0000, luwei.zhou@freescale.com wrote:
> > On Wed, Oct 1, 2014 at 11:59:00AM, Richard Cochran wrote:
> > > -----Original Message-----
> > > From: Richard Cochran [mailto:richardcochran@gmail.com]
> > > Sent: Wednesday, October 01, 2014 11:59 AM
> > > To: Zhou Luwei-B45643
> > > Cc: davem@davemloft.net; netdev@vger.kernel.org;
> > > shawn.guo@linaro.org; bhutchings@solarflare.com; Estevam
> > > Fabio-R49496; Duan Fugang-B38611; Li Frank-B20596;
> > > stephen@networkplumber.org
> > > Subject: Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts
> > > binding doc
> > >
> > > On Thu, Sep 25, 2014 at 04:43:50PM +0200, Richard Cochran wrote:
> > > > On Thu, Sep 25, 2014 at 04:10:21PM +0800, Luwei Zhou wrote:
> > > > > This patch update fec devicetree binding doc that add Optional
> > > > > properties "pps-channel".
> > > >
> > > > Again, use the PTP pin interface. We don't need a random new FEC
> > > > DT property for this.
> > >
> > > BTW, if the "channel" only means an internal timer resource, please
> > > find a proper way to claim that resource. Do not add another random
> > > DT property. We have enough of those already.
> > >
> > > Thanks,
> > > Richard
> >
> > Okay. I will use the PTP pin interface to let user set the channel
> > index as V2 patch does. Sorry for the late reply caused by vocation.
>
> Do not use the PTP pin control for an internal PPS event. Instead, just
> claim the timer as some kind of internal resource. It looks like timers
> are already part of DT, so why not use those?
>
> Thanks,
> Richard
Hi Richard,
I am not expert in DT.I didn't get your point of using timer resource in the DT. Yes, the timer resource is
Part of FEC IP hardware. But if you want to choose one channel for using PPS, you have to add another property
to specify. Do you mean the current code already have the DT property to specify the channel?
Thanks
Luwei
^ permalink raw reply
* Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding doc
From: Richard Cochran @ 2014-10-08 8:06 UTC (permalink / raw)
To: luwei.zhou@freescale.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Fabio.Estevam@freescale.com,
fugang.duan@freescale.com, Frank.Li@freescale.com,
stephen@networkplumber.org
In-Reply-To: <39bcecf4ec1a4862bc7316dc845a535c@BY2PR03MB441.namprd03.prod.outlook.com>
On Wed, Oct 08, 2014 at 03:15:08AM +0000, luwei.zhou@freescale.com wrote:
> On Wed, Oct 1, 2014 at 11:59:00AM, Richard Cochran wrote:
> > -----Original Message-----
> > From: Richard Cochran [mailto:richardcochran@gmail.com]
> > Sent: Wednesday, October 01, 2014 11:59 AM
> > To: Zhou Luwei-B45643
> > Cc: davem@davemloft.net; netdev@vger.kernel.org; shawn.guo@linaro.org;
> > bhutchings@solarflare.com; Estevam Fabio-R49496; Duan Fugang-B38611; Li
> > Frank-B20596; stephen@networkplumber.org
> > Subject: Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding
> > doc
> >
> > On Thu, Sep 25, 2014 at 04:43:50PM +0200, Richard Cochran wrote:
> > > On Thu, Sep 25, 2014 at 04:10:21PM +0800, Luwei Zhou wrote:
> > > > This patch update fec devicetree binding doc that add Optional
> > > > properties "pps-channel".
> > >
> > > Again, use the PTP pin interface. We don't need a random new FEC DT
> > > property for this.
> >
> > BTW, if the "channel" only means an internal timer resource, please find
> > a proper way to claim that resource. Do not add another random DT
> > property. We have enough of those already.
> >
> > Thanks,
> > Richard
>
> Okay. I will use the PTP pin interface to let user set the channel index as V2 patch does. Sorry for the
> late reply caused by vocation.
Do not use the PTP pin control for an internal PPS event. Instead,
just claim the timer as some kind of internal resource. It looks like
timers are already part of DT, so why not use those?
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Lothar Waßmann @ 2014-10-08 5:01 UTC (permalink / raw)
To: David Laight
Cc: 'Eric Dumazet', netdev@vger.kernel.org, David S. Miller,
Russell King, Frank Li, Fabio Estevam,
linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C6116@AcuExch.aculab.com>
Hi,
David Laight wrote:
> From: Eric Dumazet
> > On Tue, 2014-10-07 at 15:19 +0200, Lothar Wamann wrote:
> > > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > > introduced a regression for i.MX28. The swap_buffer() function doing
> > > the endian conversion of the received data on i.MX28 may access memory
> > > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > > does not copy those bytes, so that the last bytes of a packet may be
> > > filled with invalid data after swapping.
> > > This will likely lead to checksum errors on received packets.
> > > E.g. when trying to mount an NFS rootfs:
> > > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> > >
> > > Do the byte swapping and copying to the new skb in one go if
> > > necessary.
> > >
> > > Signed-off-by: Lothar Wamann <LW@KARO-electronics.de>
> > > ---
> > > drivers/net/ethernet/freescale/fec_main.c | 25 +++++++++++++++++++++----
> > > 1 file changed, 21 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> > > index 87975b5..eaaebad 100644
> > > --- a/drivers/net/ethernet/freescale/fec_main.c
> > > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > > @@ -339,6 +339,18 @@ static void *swap_buffer(void *bufaddr, int len)
> > > return bufaddr;
> > > }
> > >
> > > +static void *swap_buffer2(void *dst_buf, void *src_buf, int len)
> > > +{
> > > + int i;
> > > + unsigned int *src = src_buf;
> > > + unsigned int *dst = dst_buf;
> > > +
> > > + for (i = 0; i < DIV_ROUND_UP(len, 4); i++, src++, dst++)
> > > + *dst = cpu_to_be32(*src);
> >
> > No need for the DIV :
> >
> > for (i = 0; i < len; i += sizeof(*dst), src++, dst++)
> > *dst = cpu_to_be32(*src);
> >
> > Also are you sure both src/dst are aligned to word boundaries, or is
> > this architecture OK with possible misalignment ?
>
> I wondered about that as well.
> I wouldn't have expected ppc to support misaligned transfers, and you'd also
> want to make sure that cpu_to_be(*src) was using a byte-swapping instruction.
> Hmmm... cpu_to_be() doesn't sound like the right 'swap' macro name.
>
??? So what is cpu_to_be32() then?
The new swap function is an exact copy of the original one already in
use except for the fact that it uses distinct source and destination
buffers.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply
* Re: [PATCH] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Lothar Waßmann @ 2014-10-08 4:59 UTC (permalink / raw)
To: David Laight
Cc: netdev@vger.kernel.org, David S. Miller, Russell King, Frank Li,
Fabio Estevam, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D174C60B1@AcuExch.aculab.com>
Hi,
David Laight wrote:
> From: Lothar
> > David Laight wrote:
> > > From: Lothar Waßmann
> > > > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > > > introduced a regression for i.MX28. The swap_buffer() function doing
> > > > the endian conversion of the received data on i.MX28 may access memory
> > > > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > > > does not copy those bytes, so that the last bytes of a packet may be
> > > > filled with invalid data after swapping.
> > > > This will likely lead to checksum errors on received packets.
> > > > E.g. when trying to mount an NFS rootfs:
> > > > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> > > >
> > > > Do the byte swapping and copying to the new skb in one go if
> > > > necessary.
> > >
> > > ISTM that if you need to do the 'swap' you should copy the data regardless
> > > of the length.
> > >
> > The swap function has to look at at most 3 bytes beyond the actual
> > packet length. That is what the original swap_buffer() function does and
> > what the new function swap_buffer2(), that does the endian swapping
> > while copying to the new buffer, also does.
>
> I understood the bug.
>
> The point I was making is that if you have to do a read-write of the received
> data (to byteswap it) then you might as well always copy it into a new skb that
> is just big enough for the actual receive frame.
>
I wanted to use the least intrusive solution.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply
* Re: [PATCH/RFC] datapath: offload hooks
From: Stephen Hemminger @ 2014-10-08 4:55 UTC (permalink / raw)
To: Simon Horman
Cc: dev, netdev, Pravin Shelar, Jesse Gross, Jiri Pirko, Thomas Graf,
John Fastabend, Scott Feldman, Roopa Prabhu, Alexi Starovoitov,
Florian Fainelli, Jamal Hadi Salim, Bert van Leeuwen
In-Reply-To: <1412728851-32534-1-git-send-email-simon.horman@netronome.com>
On Wed, 8 Oct 2014 09:40:51 +0900
Simon Horman <simon.horman@netronome.com> wrote:
> +
> +struct ovs_offload_ops {
> + /* Flow offload functions */
> + /* Called when a flow entry is added to the flow table */
> + void (*flow_new)(struct sw_flow *);
> + /* Called when a flow entry is modified */
> + void (*flow_set)(struct sw_flow *);
> + /* Called when a flow entry is removed from the flow table */
> + void (*flow_del)(struct sw_flow *);
> + /* Called when flow stats are queried */
> + void (*flow_stats_get)(const struct sw_flow *, struct ovs_flow_stats *,
> + unsigned long *used, __be16 *tcp_flags);
> + /* Called when flow stats are removed */
> + void (*flow_stats_clear)(struct sw_flow *);
> +
> + /* Port offload functions */
> + /* Called when a vport is added to the datapath */
> + void (*vport_new)(struct sk_buff *, struct vport *,
> + struct vport_parms *);
> + /* Called when a vport is modified */
> + void (*vport_set)(struct sk_buff *, struct vport *);
> + /* Called when a vport is removed from the datapath */
> + void (*vport_del)(struct sk_buff *, struct vport *);
> + /* Called when vport stats are queried */
> + void (*vport_stats_get)(struct vport *, struct ovs_vport_stats *);
> + /* Called when vport stats are set */
> + void (*vport_stats_set)(struct vport *, struct ovs_vport_stats *);
> +
> + /* Datapath offload functions */
> + /* Called when the datapath is created */
> + void (*dp_new)(struct datapath *);
> + /* Called when the datapath is modified */
> + void (*dp_set)(struct datapath *);
> + /* Called when the datapath is removed */
> + void (*dp_del)(struct datapath *);
> + /* Called when the datapath stats are queried */
> + void (*dp_stats_get)(struct datapath *, struct ovs_dp_stats *);
> +};
What about using netlink and netlink notifiers for event type stuff?
Much easier to extend than all the _ops stuff and you can provide
hook for people that want to do it in user space.
^ permalink raw reply
* Re: [PATCH/RFC] datapath: offload hooks
From: Stephen Hemminger @ 2014-10-08 4:50 UTC (permalink / raw)
To: Simon Horman
Cc: dev, netdev, Pravin Shelar, Jesse Gross, Jiri Pirko, Thomas Graf,
John Fastabend, Scott Feldman, Roopa Prabhu, Alexi Starovoitov,
Florian Fainelli, Jamal Hadi Salim, Bert van Leeuwen
In-Reply-To: <1412728851-32534-1-git-send-email-simon.horman@netronome.com>
On Wed, 8 Oct 2014 09:40:51 +0900
Simon Horman <simon.horman@netronome.com> wrote:
> +struct ovs_offload_ops {
> + /* Flow offload functions */
> + /* Called when a flow entry is added to the flow table */
> + void (*flow_new)(struct sw_flow *);
> + /* Called when a flow entry is modified */
> + void (*flow_set)(struct sw_flow *);
> + /* Called when a flow entry is removed from the flow table */
> + void (*flow_del)(struct sw_flow *);
> + /* Called when flow stats are queried */
> + void (*flow_stats_get)(const struct sw_flow *, struct ovs_flow_stats *,
> + unsigned long *used, __be16 *tcp_flags);
> + /* Called when flow stats are removed */
> + void (*flow_stats_clear)(struct sw_flow *);
> +
> + /* Port offload functions */
> + /* Called when a vport is added to the datapath */
> + void (*vport_new)(struct sk_buff *, struct vport *,
> + struct vport_parms *);
> + /* Called when a vport is modified */
> + void (*vport_set)(struct sk_buff *, struct vport *);
> + /* Called when a vport is removed from the datapath */
> + void (*vport_del)(struct sk_buff *, struct vport *);
> + /* Called when vport stats are queried */
> + void (*vport_stats_get)(struct vport *, struct ovs_vport_stats *);
> + /* Called when vport stats are set */
> + void (*vport_stats_set)(struct vport *, struct ovs_vport_stats *);
> +
> + /* Datapath offload functions */
> + /* Called when the datapath is created */
> + void (*dp_new)(struct datapath *);
> + /* Called when the datapath is modified */
> + void (*dp_set)(struct datapath *);
> + /* Called when the datapath is removed */
> + void (*dp_del)(struct datapath *);
> + /* Called when the datapath stats are queried */
> + void (*dp_stats_get)(struct datapath *, struct ovs_dp_stats *);
> +}
For security, you should mark any ops type table const, so an
attacker can't find a home to poke their favorite routine into.
^ permalink raw reply
* Re: [PATCH] net: fec: fix regression on i.MX28 introduced by rx_copybreak support
From: Lothar Waßmann @ 2014-10-08 4:43 UTC (permalink / raw)
To: David Miller; +Cc: netdev, rmk+kernel, Frank.Li, fabio.estevam, linux-kernel
In-Reply-To: <20141007.131507.635415594538460008.davem@davemloft.net>
Hi,
David Miller wrote:
> From: Lothar Waßmann <LW@KARO-electronics.de>
> Date: Tue, 7 Oct 2014 15:19:37 +0200
>
> > commit 1b7bde6d659d ("net: fec: implement rx_copybreak to improve rx performance")
> > introduced a regression for i.MX28. The swap_buffer() function doing
> > the endian conversion of the received data on i.MX28 may access memory
> > beyond the actual packet size in the DMA buffer. fec_enet_copybreak()
> > does not copy those bytes, so that the last bytes of a packet may be
> > filled with invalid data after swapping.
> > This will likely lead to checksum errors on received packets.
> > E.g. when trying to mount an NFS rootfs:
> > UDP: bad checksum. From 192.168.1.225:111 to 192.168.100.73:44662 ulen 36
> >
> > Do the byte swapping and copying to the new skb in one go if
> > necessary.
> >
> > Signed-off-by: Lothar Waßmann <LW@KARO-electronics.de>
>
> Why don't you just round up the length fec_enet_copybreak() uses when
> need_swap is true? Then you will end up mimicking the original behavior
> and not require this new helper function.
>
I wanted to eliminate the need to access the buffer twice (once for
copying and once for swapping).
> And in any case I agree with Sergei that if you do retain your approach,
> the new 'swap' argument to fec_enet_copybreak() should be a 'bool'.
>
Right.
> I'm really surprised there isn't a control register bit to adjust the
> endianness of the data DMA'd to/from the network.
>
This is a "feature" of the i.MX28.
Lothar Waßmann
--
___________________________________________________________
Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996
www.karo-electronics.de | info@karo-electronics.de
___________________________________________________________
^ permalink raw reply
* Re: [PATCH net-next] i40e: skb->xmit_more support
From: Jeff Kirsher @ 2014-10-08 4:02 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, netdev, dborkman
In-Reply-To: <20141007.163730.2221137596344407031.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 599 bytes --]
On Tue, 2014-10-07 at 16:37 -0400, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 07 Oct 2014 13:30:23 -0700
>
> > From: Eric Dumazet <edumazet@google.com>
> >
> > Support skb->xmit_more in i40e is straightforward : we need to move
> > around i40e_maybe_stop_tx() call to correctly test netif_xmit_stopped()
> > before taking the decision to not kick the NIC.
> >
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>
> Intel folks, if I could get a quick review of this that would be great.
>
> Thanks.
Working on it, let you know tomorrow.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [PATCH v1 0/4] Enable FEC pps ouput
From: luwei.zhou @ 2014-10-08 3:30 UTC (permalink / raw)
To: Richard Cochran
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Fabio.Estevam@freescale.com,
fugang.duan@freescale.com, Frank.Li@freescale.com,
stephen@networkplumber.org
In-Reply-To: <20141003082341.GB4249@localhost.localdomain>
On Wed, Oct 3, 2014 at 04:24:00PM, Richard Cochran wrote:
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Friday, October 03, 2014 4:24 PM
> To: Zhou Luwei-B45643
> Cc: davem@davemloft.net; netdev@vger.kernel.org; shawn.guo@linaro.org;
> bhutchings@solarflare.com; Estevam Fabio-R49496; Duan Fugang-B38611; Li
> Frank-B20596; stephen@networkplumber.org
> Subject: Re: [PATCH v1 0/4] Enable FEC pps ouput
>
> On Thu, Sep 25, 2014 at 04:10:17PM +0800, Luwei Zhou wrote:
> > This patch does:
> > - Replace 32-bit free-running PTP timer with 31-bit.
> > - Implement hardware PTP timestamp adjustment.
> > - Enable PPS output based on hardware adjustment.
> >
> >
> > Luwei Zhou (4):
> > net: fec: ptp: Use the 31-bit ptp timer.
> > net: fec: ptp: Use hardware algorithm to adjust PTP counter.
> > net: fec: ptp: Enalbe PPS ouput based on ptp clock
> ^^^^^^ ^^^^^
> Enable output
>
> Also, it looks like you have implemented the wrong feature.
>
> The "pps" capability means that the clock provides a PPS event
> (interrupt) to the kernel.
>
> IOW, if .pps==1, then you must call ptp_clock_event() with event type
> PTP_CLOCK_PPS.
>
> Your patch set seems to be creating a periodic output and not a PPS
> kernel callback.
>
> You should clarify what you are trying to do, and then implement the
> appropriate interface in your driver.
>
> Thanks,
> Richard
Hum..The FEC IP does not support a specified channel for PPS function. So I have to use
a ptp timer output channel to implement the PPS function. That is why the software flow
looks like the periodic output but the function is PPS function. We only want the event to happen
on the per second. So I think it should be clarified to be PPS event.
Thanks
Luwei
^ permalink raw reply
* RE: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding doc
From: luwei.zhou @ 2014-10-08 3:15 UTC (permalink / raw)
To: Richard Cochran
Cc: davem@davemloft.net, netdev@vger.kernel.org, shawn.guo@linaro.org,
bhutchings@solarflare.com, Fabio.Estevam@freescale.com,
fugang.duan@freescale.com, Frank.Li@freescale.com,
stephen@networkplumber.org
In-Reply-To: <20141001035900.GA4544@netboy>
On Wed, Oct 1, 2014 at 11:59:00AM, Richard Cochran wrote:
> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Wednesday, October 01, 2014 11:59 AM
> To: Zhou Luwei-B45643
> Cc: davem@davemloft.net; netdev@vger.kernel.org; shawn.guo@linaro.org;
> bhutchings@solarflare.com; Estevam Fabio-R49496; Duan Fugang-B38611; Li
> Frank-B20596; stephen@networkplumber.org
> Subject: Re: [PATCH v1 4/4] ARM: Documentation: Update fec dts binding
> doc
>
> On Thu, Sep 25, 2014 at 04:43:50PM +0200, Richard Cochran wrote:
> > On Thu, Sep 25, 2014 at 04:10:21PM +0800, Luwei Zhou wrote:
> > > This patch update fec devicetree binding doc that add Optional
> > > properties "pps-channel".
> >
> > Again, use the PTP pin interface. We don't need a random new FEC DT
> > property for this.
>
> BTW, if the "channel" only means an internal timer resource, please find
> a proper way to claim that resource. Do not add another random DT
> property. We have enough of those already.
>
> Thanks,
> Richard
Okay. I will use the PTP pin interface to let user set the channel index as V2 patch does. Sorry for the
late reply caused by vocation.
Thanks,
Luwei
^ permalink raw reply
* RE: r8168 is needed to enter P-state: Package State6(pc6)onHaswellhardware
From: Hayes Wang @ 2014-10-08 2:35 UTC (permalink / raw)
To: Francois Romieu; +Cc: Ceriel Jacobs, nic_swsd, netdev@vger.kernel.org
In-Reply-To: <20141007201713.GB4398@zoreil.com>
Francois Romieu [mailto:romieu@fr.zoreil.com]
> Sent: Wednesday, October 08, 2014 4:17 AM
[...]
> > When enabling the ASPM, it would influence the thrpughput.
> > It is hard to
> > choose performance or power saving. Therefore, we reserve the config
> > to let the user determines it.
>
> Mmmm...
>
> How do Realtek's devices behave if Config{2, 3} + MISC registers are
> configured for ASPM whereas PCI config space registers aren't ?
The feature of the ASPM for the nic would be disabled.
ASPM is enabled when both the internal settings and
the PCI config space registers are enabled.
Best Regards,
Hayes
^ permalink raw reply
* Re: [PATCH net] bna: page allocation during interrupts to use a mempool.
From: Eric Dumazet @ 2014-10-08 1:28 UTC (permalink / raw)
To: Eric Wheeler; +Cc: Shahed Shaikh, Stephen Hemminger, netdev, Rasesh Mody
In-Reply-To: <alpine.DEB.2.00.1410071746230.3503@ware.dreamhost.com>
On Tue, 2014-10-07 at 17:48 -0700, Eric Wheeler wrote:
> Just += unmap->vector.len still did not work (same backtrace), so I've
> rebuilt with PAGE_SIZE<<2 and so far so good. I'll let it run all night
> and see if we get any problems.
Further inspection of the driver told me that unmap->vector.len should
be 16384 already. (same than PAGE_SIZE << 2)
(set at line 304, drivers/net/ethernet/brocade/bna/bnad.c)
So you might hit memory fragmentation issues.
Do you have CONFIG_COMPACTION=y in your .config ?
^ permalink raw reply
* [PATCH/RFC] datapath: offload hooks
From: Simon Horman @ 2014-10-08 0:40 UTC (permalink / raw)
To: dev, netdev
Cc: Pravin Shelar, Jesse Gross, Jiri Pirko, Thomas Graf,
John Fastabend, Scott Feldman, Roopa Prabhu, Alexi Starovoitov,
Florian Fainelli, Jamal Hadi Salim, Bert van Leeuwen,
Simon Horman
From: Bert van Leeuwen <bert.vanleeuwen@netronome.com>
Hi,
the purpose of posting this patch is to further the ongoing discussion
on hardware offloads for Open vSwitch. It reflects work at Netronome to
provide such offloads. The intention is for this work to be generic.
There is an accompanying document to this patch which attempts to
describe the problem-space the hooks operate in. The document
is intended to be fairly generic and high level. It is intended
to be a point for ongoing discussion and as such comments via
the Google-docs interface are welcome. It is not intended to be
Netronome specific.
https://docs.google.com/a/netronome.com/document/d/1UtG3xY0xMzIwMG732gaypyR6vxGxinkRrnMLnVS4Zao/edit#
The document at the URL above is indented to supplement the Open vSwitch
Hardware Offload Architecture document, which I believe is collated by
Thomas Graf.
https://docs.google.com/document/d/195waUliu7G5YYVuXHmLmHgJ38DFSte321WPq0oaFhyU/edit#
This patch is the collective work by a number of engineers at Netronome.
Co-authored-by: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Signed-off-by: Bert van Leeuwen <bert.vanleeuwen@netronome.com>
Signed-off-by: Simon Horman <simon.horman@netronome.com>
---
datapath/Modules.mk | 2 +
datapath/datapath.c | 22 ++++++
datapath/datapath.h | 4 +
datapath/flow.h | 1 +
datapath/linux/.gitignore | 1 +
datapath/offload_ops.c | 49 ++++++++++++
datapath/offload_ops.h | 184 ++++++++++++++++++++++++++++++++++++++++++++++
datapath/vport.h | 3 +
8 files changed, 266 insertions(+)
create mode 100644 datapath/offload_ops.c
create mode 100644 datapath/offload_ops.h
diff --git a/datapath/Modules.mk b/datapath/Modules.mk
index 90e158c..3286621 100644
--- a/datapath/Modules.mk
+++ b/datapath/Modules.mk
@@ -7,6 +7,7 @@ build_modules = $(both_modules) # Modules to build
dist_modules = $(both_modules) # Modules to distribute
openvswitch_sources = \
+ offload_ops.c \
actions.c \
datapath.c \
dp_notify.c \
@@ -22,6 +23,7 @@ openvswitch_sources = \
vport-vxlan.c
openvswitch_headers = \
+ offload_ops.h \
compat.h \
datapath.h \
flow.h \
diff --git a/datapath/datapath.c b/datapath/datapath.c
index 3ca9716..b598f48 100644
--- a/datapath/datapath.c
+++ b/datapath/datapath.c
@@ -61,6 +61,7 @@
#include "vlan.h"
#include "vport-internal_dev.h"
#include "vport-netdev.h"
+#include "offload_ops.h"
int ovs_net_id __read_mostly;
@@ -651,6 +652,7 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats,
stats->n_lost += local_stats.n_lost;
mega_stats->n_mask_hit += local_stats.n_mask_hit;
}
+ ovs_offload_dp_stats_get(dp, stats);
}
static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
@@ -705,6 +707,7 @@ static int ovs_flow_cmd_fill_stats(const struct sw_flow *flow,
unsigned long used;
ovs_flow_stats_get(flow, &stats, &used, &tcp_flags);
+ ovs_offload_flow_stats_get(flow, &stats, &used, &tcp_flags);
if (used &&
nla_put_u64(skb, OVS_FLOW_ATTR_USED, ovs_flow_used_time(used)))
@@ -906,6 +909,7 @@ static int ovs_flow_cmd_new(struct sk_buff *skb, struct genl_info *info)
acts = NULL;
goto err_unlock_ovs;
}
+ ovs_offload_flow_new(new_flow);
if (unlikely(reply)) {
error = ovs_flow_cmd_fill_info(dp, new_flow,
@@ -1059,6 +1063,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
OVS_FLOW_CMD_NEW);
BUG_ON(error < 0);
}
+ ovs_offload_flow_set(flow);
} else {
/* Could not alloc without acts before locking. */
reply = ovs_flow_cmd_build_info(dp, flow,
@@ -1073,6 +1078,7 @@ static int ovs_flow_cmd_set(struct sk_buff *skb, struct genl_info *info)
/* Clear stats. */
if (a[OVS_FLOW_ATTR_CLEAR])
ovs_flow_stats_clear(flow);
+ ovs_offload_flow_stats_clear(flow);
ovs_unlock();
if (reply)
@@ -1172,6 +1178,7 @@ static int ovs_flow_cmd_del(struct sk_buff *skb, struct genl_info *info)
goto unlock;
}
+ ovs_offload_flow_del(flow);
ovs_flow_tbl_remove(&dp->table, flow);
ovs_unlock();
@@ -1461,6 +1468,7 @@ static int ovs_dp_cmd_new(struct sk_buff *skb, struct genl_info *info)
goto err_destroy_ports_array;
}
+ ovs_offload_dp_new(dp);
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
info->snd_seq, 0, OVS_DP_CMD_NEW);
BUG_ON(err < 0);
@@ -1530,6 +1538,7 @@ static int ovs_dp_cmd_del(struct sk_buff *skb, struct genl_info *info)
if (IS_ERR(dp))
goto err_unlock_free;
+ ovs_offload_dp_del(dp);
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
info->snd_seq, 0, OVS_DP_CMD_DEL);
BUG_ON(err < 0);
@@ -1563,6 +1572,7 @@ static int ovs_dp_cmd_set(struct sk_buff *skb, struct genl_info *info)
goto err_unlock_free;
ovs_dp_change(dp, info->attrs);
+ ovs_offload_dp_set(dp);
err = ovs_dp_cmd_fill_info(dp, reply, info->snd_portid,
info->snd_seq, 0, OVS_DP_CMD_NEW);
@@ -1695,6 +1705,7 @@ static int ovs_vport_cmd_fill_info(struct vport *vport, struct sk_buff *skb,
goto nla_put_failure;
ovs_vport_get_stats(vport, &vport_stats);
+ ovs_offload_vport_stats_get(vport, &vport_stats);
if (nla_put(skb, OVS_VPORT_ATTR_STATS, sizeof(struct ovs_vport_stats),
&vport_stats))
goto nla_put_failure;
@@ -1833,10 +1844,14 @@ static int ovs_vport_cmd_new(struct sk_buff *skb, struct genl_info *info)
err = 0;
if (a[OVS_VPORT_ATTR_STATS])
ovs_vport_set_stats(vport, nla_data(a[OVS_VPORT_ATTR_STATS]));
+ ovs_offload_vport_stats_set(vport,
+ nla_data(a[OVS_VPORT_ATTR_STATS]));
+
err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
info->snd_seq, 0, OVS_VPORT_CMD_NEW);
BUG_ON(err < 0);
+ ovs_offload_vport_new(skb, vport, &parms);
ovs_unlock();
ovs_notify(&dp_vport_genl_family, &ovs_dp_vport_multicast_group, reply, info);
@@ -1891,6 +1906,7 @@ static int ovs_vport_cmd_set(struct sk_buff *skb, struct genl_info *info)
err = ovs_vport_cmd_fill_info(vport, reply, info->snd_portid,
info->snd_seq, 0, OVS_VPORT_CMD_NEW);
BUG_ON(err < 0);
+ ovs_offload_vport_set(skb, vport);
ovs_unlock();
ovs_notify(&dp_vport_genl_family, &ovs_dp_vport_multicast_group, reply, info);
@@ -1928,6 +1944,7 @@ static int ovs_vport_cmd_del(struct sk_buff *skb, struct genl_info *info)
info->snd_seq, 0, OVS_VPORT_CMD_DEL);
BUG_ON(err < 0);
ovs_dp_detach_port(vport);
+ ovs_offload_vport_del(skb, vport);
ovs_unlock();
ovs_notify(&dp_vport_genl_family, &ovs_dp_vport_multicast_group, reply, info);
@@ -2149,6 +2166,10 @@ static int __init dp_init(void)
if (err < 0)
goto error_unreg_notifier;
+ err = ovs_offload_init_handler();
+ if (err)
+ goto error;
+
return 0;
error_unreg_notifier:
@@ -2165,6 +2186,7 @@ error:
static void dp_cleanup(void)
{
+ ovs_offload_cleanup_handler();
dp_unregister_genl(ARRAY_SIZE(dp_genl_families));
unregister_netdevice_notifier(&ovs_dp_device_notifier);
unregister_pernet_device(&ovs_net_ops);
diff --git a/datapath/datapath.h b/datapath/datapath.h
index 651c119..fc54169 100644
--- a/datapath/datapath.h
+++ b/datapath/datapath.h
@@ -68,6 +68,7 @@ struct dp_stats_percpu {
* @ports: Hash table for ports. %OVSP_LOCAL port always exists. Protected by
* ovs_mutex and RCU.
* @stats_percpu: Per-CPU datapath statistics.
+ * @offload: Hardware offload callbacks
* @net: Reference to net namespace.
*
* Context: See the comment on locking at the top of datapath.c for additional
@@ -86,6 +87,9 @@ struct datapath {
/* Stats. */
struct dp_stats_percpu __percpu *stats_percpu;
+ /* Hardware offload ptr. */
+ void *offload;
+
#ifdef CONFIG_NET_NS
/* Network namespace ref. */
struct net *net;
diff --git a/datapath/flow.h b/datapath/flow.h
index 44ed10d..e50988c 100644
--- a/datapath/flow.h
+++ b/datapath/flow.h
@@ -220,6 +220,7 @@ struct sw_flow {
int stats_last_writer; /* NUMA-node id of the last writer on
* 'stats[0]'.
*/
+ void *offload;
struct sw_flow_key key;
struct sw_flow_key unmasked_key;
struct sw_flow_mask *mask;
diff --git a/datapath/linux/.gitignore b/datapath/linux/.gitignore
index be233fc..0ce4663 100644
--- a/datapath/linux/.gitignore
+++ b/datapath/linux/.gitignore
@@ -51,3 +51,4 @@
/vport.c
/vxlan.c
/workqueue.c
+/offload_ops.c
diff --git a/datapath/offload_ops.c b/datapath/offload_ops.c
new file mode 100644
index 0000000..2eda940
--- /dev/null
+++ b/datapath/offload_ops.c
@@ -0,0 +1,49 @@
+#include "offload_ops.h"
+
+struct ovs_offload_ops *offload_ops;
+
+int ovs_offload_register(const struct ovs_offload_ops *new_handler)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload)
+ return -EBUSY;
+ offload = kmalloc(sizeof(*offload), GFP_KERNEL);
+ if (!offload)
+ return -ENOMEM;
+ *offload = *new_handler;
+
+ rcu_assign_pointer(offload_ops, offload);
+ return 0;
+}
+
+EXPORT_SYMBOL(ovs_offload_register);
+
+int ovs_offload_unregister()
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload) {
+ rcu_assign_pointer(offload_ops, NULL);
+ kfree(offload);
+ }
+ return 0;
+}
+
+EXPORT_SYMBOL(ovs_offload_unregister);
+
+int ovs_offload_init_handler(void)
+{
+ rcu_assign_pointer(offload_ops, NULL);
+ return 0;
+}
+
+void ovs_offload_cleanup_handler(void)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload)
+ kfree(offload);
+ rcu_assign_pointer(offload_ops, NULL);
+}
+
diff --git a/datapath/offload_ops.h b/datapath/offload_ops.h
new file mode 100644
index 0000000..9783ba4
--- /dev/null
+++ b/datapath/offload_ops.h
@@ -0,0 +1,184 @@
+#ifndef _OFFLOAD_OPS_H
+#define _OFFLOAD_OPS_H
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+
+#include "datapath.h"
+#include "flow.h"
+
+struct ovs_offload_ops {
+ /* Flow offload functions */
+ /* Called when a flow entry is added to the flow table */
+ void (*flow_new)(struct sw_flow *);
+ /* Called when a flow entry is modified */
+ void (*flow_set)(struct sw_flow *);
+ /* Called when a flow entry is removed from the flow table */
+ void (*flow_del)(struct sw_flow *);
+ /* Called when flow stats are queried */
+ void (*flow_stats_get)(const struct sw_flow *, struct ovs_flow_stats *,
+ unsigned long *used, __be16 *tcp_flags);
+ /* Called when flow stats are removed */
+ void (*flow_stats_clear)(struct sw_flow *);
+
+ /* Port offload functions */
+ /* Called when a vport is added to the datapath */
+ void (*vport_new)(struct sk_buff *, struct vport *,
+ struct vport_parms *);
+ /* Called when a vport is modified */
+ void (*vport_set)(struct sk_buff *, struct vport *);
+ /* Called when a vport is removed from the datapath */
+ void (*vport_del)(struct sk_buff *, struct vport *);
+ /* Called when vport stats are queried */
+ void (*vport_stats_get)(struct vport *, struct ovs_vport_stats *);
+ /* Called when vport stats are set */
+ void (*vport_stats_set)(struct vport *, struct ovs_vport_stats *);
+
+ /* Datapath offload functions */
+ /* Called when the datapath is created */
+ void (*dp_new)(struct datapath *);
+ /* Called when the datapath is modified */
+ void (*dp_set)(struct datapath *);
+ /* Called when the datapath is removed */
+ void (*dp_del)(struct datapath *);
+ /* Called when the datapath stats are queried */
+ void (*dp_stats_get)(struct datapath *, struct ovs_dp_stats *);
+};
+
+extern struct ovs_offload_ops *offload_ops;
+
+int ovs_offload_register(const struct ovs_offload_ops *new_handler);
+int ovs_offload_unregister(void);
+int ovs_offload_init_handler(void);
+void ovs_offload_cleanup_handler(void);
+
+/* Wrappers for calling offload function with locking in place */
+/* Flow offload functions */
+static inline void ovs_offload_flow_new(struct sw_flow *flow)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->flow_new)
+ offload->flow_new(flow);
+}
+
+static inline void ovs_offload_flow_set(struct sw_flow *flow)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->flow_set)
+ offload->flow_set(flow);
+}
+
+static inline void ovs_offload_flow_del(struct sw_flow *flow)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->flow_del)
+ offload->flow_del(flow);
+}
+
+static inline void ovs_offload_flow_stats_get(const struct sw_flow *flow,
+ struct ovs_flow_stats *stats,
+ unsigned long *used,
+ __be16 *tcp_flags)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->flow_stats_get)
+ offload->flow_stats_get(flow, stats, used, tcp_flags);
+}
+
+static inline void ovs_offload_flow_stats_clear(struct sw_flow *flow)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->flow_stats_clear)
+ offload->flow_stats_clear(flow);
+}
+
+/* Port offload functions */
+static inline void ovs_offload_vport_new(struct sk_buff *skb,
+ struct vport *vport,
+ struct vport_parms *parms)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->vport_new)
+ offload->vport_new(skb, vport, parms);
+}
+
+static inline void ovs_offload_vport_set(struct sk_buff *skb,
+ struct vport *vport)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->vport_set)
+ offload->vport_set(skb, vport);
+}
+
+static inline void ovs_offload_vport_del(struct sk_buff *skb,
+ struct vport *vport)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->vport_del)
+ offload->vport_del(skb, vport);
+}
+
+static inline void
+ovs_offload_vport_stats_get(struct vport *vport,
+ struct ovs_vport_stats *vport_stats)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->vport_stats_get)
+ offload->vport_stats_get(vport, vport_stats);
+}
+
+static inline void
+ovs_offload_vport_stats_set(struct vport *vport,
+ struct ovs_vport_stats *vport_stats)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->vport_stats_set)
+ offload->vport_stats_set(vport, vport_stats);
+}
+
+/* Datapath offload functions */
+static inline void ovs_offload_dp_new(struct datapath *dp)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->dp_new)
+ offload->dp_new(dp);
+}
+
+static inline void ovs_offload_dp_set(struct datapath *dp)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->dp_set)
+ offload->dp_set(dp);
+}
+
+static inline void ovs_offload_dp_del(struct datapath *dp)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->dp_del)
+ offload->dp_del(dp);
+}
+
+static inline void ovs_offload_dp_stats_get(struct datapath *dp,
+ struct ovs_dp_stats *stats)
+{
+ struct ovs_offload_ops *offload = rcu_dereference(offload_ops);
+
+ if (offload && offload->dp_stats_get)
+ offload->dp_stats_get(dp, stats);
+}
+
+#endif /* offload_ops.h */
diff --git a/datapath/vport.h b/datapath/vport.h
index 8c3da05..a2f2719 100644
--- a/datapath/vport.h
+++ b/datapath/vport.h
@@ -104,6 +104,7 @@ struct vport_portids {
* @stats_lock: Protects @err_stats and @offset_stats.
* @err_stats: Points to error statistics used and maintained by vport
* @offset_stats: Added to actual statistics as a sop to compatibility with
+ * @offload: Hardware offload callbacks
* XAPI for Citrix XenServer. Deprecated.
*/
struct vport {
@@ -121,6 +122,8 @@ struct vport {
spinlock_t stats_lock;
struct vport_err_stats err_stats;
struct ovs_vport_stats offset_stats;
+
+ void *offload;
};
/**
--
2.1.1
^ permalink raw reply related
* Re: [net v2 0/8] gianfar: ARM port driver updates (1/2)
From: David Miller @ 2014-10-08 0:35 UTC (permalink / raw)
To: claudiu.manoil; +Cc: netdev, Li.Xiubo, Shruti, kim.phillips
In-Reply-To: <1412667875-18225-1-git-send-email-claudiu.manoil@freescale.com>
From: Claudiu Manoil <claudiu.manoil@freescale.com>
Date: Tue, 7 Oct 2014 10:44:27 +0300
> This is the first round of driver portability fixes and clean-up
> with the main purpose to make gianfar portable on ARM, for the ARM
> based SoC that integrates the eTSEC ethernet controller - "ls1021a".
> The patches primarily address compile time errors, when compiling
> gianfar on ARM. They replace PPC specific functions and macros
> with architecture independent ones, solve arch specific header
> inclusions, guard code that relates to PPC only, and even address
> some simple endianess issues (see MAC address setup patch).
> The patches addressing the bulk of remaining endianess issues,
> like handling DMA fields (BD and FCB), will follow with the second
> round.
>
> Reviewed-by: Kim Phillips <kim.phillips@freescale.com>
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net] bna: page allocation during interrupts to use a mempool.
From: Eric Wheeler @ 2014-10-08 0:48 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Shahed Shaikh, Stephen Hemminger, netdev, Rasesh Mody
In-Reply-To: <1412651746.11091.98.camel@edumazet-glaptop2.roam.corp.google.com>
On Mon, 6 Oct 2014, Eric Dumazet wrote:
> On Mon, 2014-10-06 at 20:05 -0700, Eric Wheeler wrote:
>> On Mon, 6 Oct 2014, Eric Dumazet wrote:
>>> On Mon, 2014-10-06 at 18:57 -0700, Eric Wheeler wrote:
>>>> This patch fixes an order:2 memory allocation error backtrace by
>>>> guaranteeing that memory is available during simultaneous high memory
>>>> pressure and packet rates when using 9k jumbo frames.
>>>
>>> This is highly suspect to me.
>>> Most likely yet another truesize lie.
>>> At a first glance, bnad_cq_setup_skb_frags() is buggy here :
>>> skb->truesize += totlen;
>
> It seems many drivers make this assumption that a frame of 1000 bytes
> consumes 1000 bytes of memory.
>
> Reality is that driver allocated more memory, because it can not predict
> how many bytes are going to be received from the network.
>
> By lying on skb->truesize (underestimating real memory cost), this
> prevents networking stack making appropriate memory checks.
>
> Here, it seems clear to me the following fix is needed, at very minimum.
>
> And it might be that something better is needed : MTU=9000 might force
> the driver to allocate 16384 bytes per frame, not 9018
>
> So it is possible that unmap->vector.len needs to be changed to the real
> size of memory region (for example : PAGE_SIZE << 2)
Just += unmap->vector.len still did not work (same backtrace), so I've
rebuilt with PAGE_SIZE<<2 and so far so good. I'll let it run all night
and see if we get any problems.
-Eric
--
Eric Wheeler, President eWheeler, Inc. dba Global Linux Security
888-LINUX26 (888-546-8926) Fax: 503-716-3878 PO Box 25107
www.GlobalLinuxSecurity.pro Linux since 1996! Portland, OR 97298
>
>
> diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
> index ffc92a41d75be550d27698af6ca3e600d9a146fe..ce867219e2ceaf33b17595b67bae99e964d5a6b6 100644
> --- a/drivers/net/ethernet/brocade/bna/bnad.c
> +++ b/drivers/net/ethernet/brocade/bna/bnad.c
> @@ -550,6 +550,7 @@ bnad_cq_setup_skb_frags(struct bna_rcb *rcb, struct sk_buff *skb,
> dma_unmap_addr(&unmap->vector, dma_addr),
> unmap->vector.len, DMA_FROM_DEVICE);
>
> + skb->truesize += unmap->vector.len;
> len = (vec == nvecs) ?
> last_fraglen : unmap->vector.len;
> totlen += len;
> @@ -563,7 +564,6 @@ bnad_cq_setup_skb_frags(struct bna_rcb *rcb, struct sk_buff *skb,
>
> skb->len += totlen;
> skb->data_len += totlen;
> - skb->truesize += totlen;
> }
>
> static inline void
>
>
>
> --
> 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 1/1 net-next] netlabel: kernel-doc warning fix
From: David Miller @ 2014-10-08 0:30 UTC (permalink / raw)
To: paul; +Cc: fabf, linux-kernel, netdev
In-Reply-To: <18856194.K9PbrNbEaI@sifl>
From: Paul Moore <paul@paul-moore.com>
Date: Tue, 07 Oct 2014 18:13:34 -0400
> On Tuesday, October 07, 2014 10:31:32 PM Fabian Frederick wrote:
>> no secid argument in netlbl_cfg_unlbl_static_del
>>
>> Signed-off-by: Fabian Frederick <fabf@skynet.be>
>
> Thanks. Likely a copy/paste error from netlbl_cfg_unlbl_static_add().
>
> Acked-by: Paul Moore <paul@paul-moore.com>
Applied, thanks everyone.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] wimax: convert printk to pr_foo()
From: David Miller @ 2014-10-08 0:27 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, inaky.perez-gonzalez, linux-wimax, netdev
In-Reply-To: <1412712724-4100-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 7 Oct 2014 22:12:03 +0200
> Use current logging functions and add module name prefix.
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Tom Herbert @ 2014-10-07 23:43 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David Miller, Jesse Gross, gerlitz.or@gmail.com, Alexander Duyck,
John Fastabend, Jeff Kirsher, netdev@vger.kernel.org, Thomas Graf,
Pravin Shelar, Andy Zhou
In-Reply-To: <CAADnVQ+wRjo3ORK5_Ra4c9W5bgyFBgotbeGB1fAuPkz1a=oy7g@mail.gmail.com>
On Tue, Oct 7, 2014 at 3:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Tue, Oct 7, 2014 at 1:48 PM, David Miller <davem@davemloft.net> wrote:
>>> They're exposing packet parsers to users, so we will be able to
>>> program any protocol into the device without reflashing it.
>>> Some of the guys are even allowing reprogramming the parser
>>> while packets are flowing.
>>
>> So we have to write new software in _EVERY_ driver to accomodate this.
>>
>> That makes zero sense either, and it is unneeded complexity in the
>> hardware.
>
> I have to agree that for single purpose of csum verify
> adding all the complexity around programmable
> parsers doesn't make sense.
> To me packet parsing is the first step of HW offload,
> regardless whether it's flow based or what else this hw can do.
>
> I'd rather see HW vendors provide programmable parser
> for any and all protocols instead of them saying:
> here is my device X that supports protocols defined
> by standards Y and Z.
> (which is the case today and it's not pretty)
>
>> COMPLETE works everywhere, on everything, with no driver changes, and
>> is so much harder to get wrong.
>
> agree. I'm not against COMPLETE in any way.
>
>> Every protocol specific feature has major downsides whether you choose
>> to see them or not.
>
> I surely don't have a preference to a protocol.
> I want to see programmable HW that supports any crazy
> packet format.
>
> Let's get back to COMPLETE and VMs example,
> because I want to see it fixed.
> In such case kernel stack in hypervisor will be pulling
> encap headers then populating new field in virtio
> ring with updated csum and let guest VMs to continue
> adjusting that csum, right?
That seems like the correct approach. It's not even guaranteed that
the kernel or device could parse the inner packet to verify inner
checksums (consider if checksum was added to a protocol like QUIC).
> Alternative would be to do inner processing and
> csum verification in hypervisor so that packet
> can be marked as DATA_VALID, but is it better?
One important clarification in RX checksum overhaul is that
CHECKSUM_UNNECESSARY does not mean that all possible checksums in the
packet have been verified. It means that one or more (per csum_level)
consecutive checksums have been verified. Seems like the DATA_VALID
interface might need a similar clarification.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] netlabel: kernel-doc warning fix
From: Paul Moore @ 2014-10-07 22:13 UTC (permalink / raw)
To: Fabian Frederick, David S. Miller; +Cc: linux-kernel, netdev
In-Reply-To: <1412713892-5537-1-git-send-email-fabf@skynet.be>
On Tuesday, October 07, 2014 10:31:32 PM Fabian Frederick wrote:
> no secid argument in netlbl_cfg_unlbl_static_del
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
Thanks. Likely a copy/paste error from netlbl_cfg_unlbl_static_add().
Acked-by: Paul Moore <paul@paul-moore.com>
> ---
> net/netlabel/netlabel_kapi.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
> index 0b4692d..a845cd4 100644
> --- a/net/netlabel/netlabel_kapi.c
> +++ b/net/netlabel/netlabel_kapi.c
> @@ -246,7 +246,6 @@ int netlbl_cfg_unlbl_static_add(struct net *net,
> * @addr: IP address in network byte order (struct in[6]_addr)
> * @mask: address mask in network byte order (struct in[6]_addr)
> * @family: address family
> - * @secid: LSM secid value for the entry
> * @audit_info: NetLabel audit information
> *
> * Description:
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Alexei Starovoitov @ 2014-10-07 22:05 UTC (permalink / raw)
To: David Miller
Cc: Tom Herbert, Jesse Gross, gerlitz.or@gmail.com, Alexander Duyck,
John Fastabend, Jeff Kirsher, netdev@vger.kernel.org, Thomas Graf,
Pravin Shelar, Andy Zhou
On Tue, Oct 7, 2014 at 1:48 PM, David Miller <davem@davemloft.net> wrote:
>> They're exposing packet parsers to users, so we will be able to
>> program any protocol into the device without reflashing it.
>> Some of the guys are even allowing reprogramming the parser
>> while packets are flowing.
>
> So we have to write new software in _EVERY_ driver to accomodate this.
>
> That makes zero sense either, and it is unneeded complexity in the
> hardware.
I have to agree that for single purpose of csum verify
adding all the complexity around programmable
parsers doesn't make sense.
To me packet parsing is the first step of HW offload,
regardless whether it's flow based or what else this hw can do.
I'd rather see HW vendors provide programmable parser
for any and all protocols instead of them saying:
here is my device X that supports protocols defined
by standards Y and Z.
(which is the case today and it's not pretty)
> COMPLETE works everywhere, on everything, with no driver changes, and
> is so much harder to get wrong.
agree. I'm not against COMPLETE in any way.
> Every protocol specific feature has major downsides whether you choose
> to see them or not.
I surely don't have a preference to a protocol.
I want to see programmable HW that supports any crazy
packet format.
Let's get back to COMPLETE and VMs example,
because I want to see it fixed.
In such case kernel stack in hypervisor will be pulling
encap headers then populating new field in virtio
ring with updated csum and let guest VMs to continue
adjusting that csum, right?
Alternative would be to do inner processing and
csum verification in hypervisor so that packet
can be marked as DATA_VALID, but is it better?
^ permalink raw reply
* Re: [PATCH net-next] i40e: skb->xmit_more support
From: Daniel Borkmann @ 2014-10-07 22:03 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Jeff Kirsher
In-Reply-To: <1412713823.11091.166.camel@edumazet-glaptop2.roam.corp.google.com>
On 10/07/2014 10:30 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Support skb->xmit_more in i40e is straightforward : we need to move
> around i40e_maybe_stop_tx() call to correctly test netif_xmit_stopped()
> before taking the decision to not kick the NIC.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Jeff, feel free to drop the same version I've sent you yesterday
directly since this here is already in patchwork ...
Thanks, Eric!
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
^ permalink raw reply
* Re: [PATCH] net: Add ndo_gso_check
From: Thomas Graf @ 2014-10-07 21:41 UTC (permalink / raw)
To: David Miller
Cc: alexei.starovoitov, therbert, jesse, gerlitz.or,
alexander.h.duyck, john.r.fastabend, jeffrey.t.kirsher, netdev,
pshelar, azhou
In-Reply-To: <20141007.164802.624713385845633192.davem@davemloft.net>
On 10/07/14 at 04:48pm, David Miller wrote:
> So we have to write new software in _EVERY_ driver to accomodate this.
>
> That makes zero sense either, and it is unneeded complexity in the
> hardware.
>
> COMPLETE works everywhere, on everything, with no driver changes, and
> is so much harder to get wrong.
>
> Every protocol specific feature has major downsides whether you choose
> to see them or not.
It's probably not overly bold to state that the behaviour behind
CHECKSUM_UNNECESSARY was not created for technical reasons in the
first place.
I enourage NICs to have protocol awareness in the absence of full
programmability but I agree with Dave that there is little to no
reason to expose anything but the full packet checksum to the CPU.
^ permalink raw reply
* Re: [PATCH 1/1 net-next] af_unix: remove NULL assignment on static
From: Fabian Frederick @ 2014-10-07 21:05 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: netdev, linux-kernel, Guenter Roeck, David Miller
In-Reply-To: <1412715283.11600.11.camel@localhost>
> On 07 October 2014 at 22:54 Hannes Frederic Sowa <hannes@stressinduktion.org>
> wrote:
>
>
> On Di, 2014-10-07 at 22:49 +0200, Fabian Frederick wrote:
> >
> > > On 07 October 2014 at 22:33 Guenter Roeck <linux@roeck-us.net> wrote:
> > >
> > >
> > > On Tue, Oct 07, 2014 at 04:18:32PM -0400, David Miller wrote:
> > > > From: Fabian Frederick <fabf@skynet.be>
> > > > Date: Tue, 7 Oct 2014 22:16:36 +0200
> > > >
> > > > > static values are automatically initialized to NULL
> > > > >
> > > > > Signed-off-by: Fabian Frederick <fabf@skynet.be>
> > > >
> > > > Isn't there some implementation room given to compilers
> > > > as to the representation of true and false?
> > >
> > > Not for true/false.
> > >
> > > C99 standard, section 7.16:
> > >
> > > ...
> > > The remaining three macros are suitable for use in #if preprocessing
> > > directives. They are
> > >
> > > true
> > >
> > > which expands to the integer constant 1,
> > >
> > > false
> > >
> > > which expands to the integer constant 0, and
> > > ...
> > >
> > > No idea where the NULL comes into the picture, though.
> > >
> > > Guenter
> >
> > Maybe comment should have been "static values are automatically initialized
> > to
> > 0" then ?
>
> I think David's concern was whether if 0 == false in all situations. It
> is pretty clear that static memory is initialized to 0.
>
> Thanks,
> Hannes
Of course :) It was an answer to Guenter's explanation.
Regards,
Fabian
>
>
^ permalink raw reply
* Re: [PATCH V2 net-next] af_unix: remove 0 assignment on static
From: David Miller @ 2014-10-07 21:03 UTC (permalink / raw)
To: fabf; +Cc: linux-kernel, netdev
In-Reply-To: <1412715735-6757-1-git-send-email-fabf@skynet.be>
From: Fabian Frederick <fabf@skynet.be>
Date: Tue, 7 Oct 2014 23:02:15 +0200
> static values are automatically initialized to 0
>
> Signed-off-by: Fabian Frederick <fabf@skynet.be>
> ---
> V2: replace NULL by 0 in description
Applied, thanks Fabian.
^ 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