* Re: [PATCH] flowcache: Increase threshold for refusing new allocations
From: Steffen Klassert @ 2016-11-23 12:42 UTC (permalink / raw)
To: Miroslav Urbanek; +Cc: NetDev
In-Reply-To: <20161121144820.GA4048@miroslavurbanek.com>
On Mon, Nov 21, 2016 at 03:48:21PM +0100, Miroslav Urbanek wrote:
> The threshold for OOM protection is too small for systems with large
> number of CPUs. Applications report ENOBUFs on connect() every 10
> minutes.
>
> The problem is that the variable net->xfrm.flow_cache_gc_count is a
> global counter while the variable fc->high_watermark is a per-CPU
> constant. Take the number of CPUs into account as well.
>
> Fixes: 6ad3122a08e3 ("flowcache: Avoid OOM condition under preasure")
> Reported-by: Lukáš Koldrt <lk@excello.cz>
> Tested-by: Jan Hejl <jh@excello.cz>
> Signed-off-by: Miroslav Urbanek <mu@miroslavurbanek.com>
Applied to the ipsec tree, thanks!
^ permalink raw reply
* RE: [PATCH] net: fix bogus cast in skb_pagelen() and use unsigned variables
From: David Laight @ 2016-11-23 12:49 UTC (permalink / raw)
To: 'Alexey Dobriyan', davem@davemloft.net; +Cc: netdev@vger.kernel.org
In-Reply-To: <20161119010808.GF1200@avx2>
From: Alexey Dobriyan
> Sent: 19 November 2016 01:08
...
> - for (i = (int)skb_shinfo(skb)->nr_frags - 1; i >= 0; i--)
> + for (i = skb_shinfo(skb)->nr_frags - 1; (int)i >= 0; i--)
> len += skb_frag_size(&skb_shinfo(skb)->frags[i]);
Think I'd use:
for (i = skb_shinfo(skb)->nr_frags; i-- != 0; )
David
^ permalink raw reply
* Re: [PATCH net-next 1/4] net: mvneta: Convert to be 64 bits compatible
From: Gregory CLEMENT @ 2016-11-23 13:07 UTC (permalink / raw)
To: Jisheng Zhang, Arnd Bergmann
Cc: linux-arm-kernel, Thomas Petazzoni, Andrew Lunn, Jason Cooper,
netdev, linux-kernel, Marcin Wojtas, David S. Miller,
Sebastian Hesselbarth
In-Reply-To: <9432400.S1OrxC027t@wuerfel>
Hi Jisheng, Arnd,
Thanks for your feedback.
On mer., nov. 23 2016, Arnd Bergmann <arnd@arndb.de> wrote:
> On Wednesday, November 23, 2016 5:53:41 PM CET Jisheng Zhang wrote:
>> On Tue, 22 Nov 2016 22:04:12 +0100 Arnd Bergmann wrote:
>>
>> > On Tuesday, November 22, 2016 5:48:41 PM CET Gregory CLEMENT wrote:
>> > > +#ifdef CONFIG_64BIT
>> > > + void *data_tmp;
>> > > +
>> > > + /* In Neta HW only 32 bits data is supported, so in order to
>> > > + * obtain whole 64 bits address from RX descriptor, we store
>> > > + * the upper 32 bits when allocating buffer, and put it back
>> > > + * when using buffer cookie for accessing packet in memory.
>> > > + * Frags should be allocated from single 'memory' region,
>> > > + * hence common upper address half should be sufficient.
>> > > + */
>> > > + data_tmp = mvneta_frag_alloc(pp->frag_size);
>> > > + if (data_tmp) {
>> > > + pp->data_high = (u64)upper_32_bits((u64)data_tmp) << 32;
>> > > + mvneta_frag_free(pp->frag_size, data_tmp);
>> > > + }
>> > >
>> >
>> > How does this work when the region spans a n*4GB address boundary?
>>
>> indeed. We also make use of this driver on 64bit platforms. We use
>> different solution to make the driver 64bit safe.
>>
>> solA: make use of the reserved field in the mvneta_rx_desc, such
>> as reserved2 etc. Yes, the field is marked as "for future use, PnC", but
>> now it's not used at all. This is one possible solution however.
>
> Right, this sounds like the most straightforward choice.
The PnC (which stands for Parsing and Classification) is not used yet
indeed but this field will be needed when we will enable it. It is
something we want to do but it is not planned in a near future. However
from the datasheets I have it seems only present on the Armada XP. It is
not mentioned on datasheets for the Armada 38x or the Armada 3700.
That would mean it was safe to use on of this field in 64-bits mode on
the Armada 3700.
So I am going to take this approach.
Thanks,
Gregory
>
>> solB: allocate a shadow buf cookie during init, e.g
>>
>> rxq->descs_bufcookie = kmalloc(rxq->size * sizeof(void*), GFP_KERNEL);
>>
>> then modify mvneta_rx_desc_fill a bit to save the 64bit pointer in
>> the shadow buf cookie, e.g
>> static void mvneta_rx_desc_fill(struct mvneta_rx_desc *rx_desc,
>> u32 phys_addr, u32 cookie,
>> struct mvneta_rx_queue *rxq)
>>
>> {
>> int i;
>>
>> rx_desc->buf_cookie = cookie;
>> rx_desc->buf_phys_addr = phys_addr;
>> i = rx_desc - rxq->descs;
>> rxq->descs_bufcookie[i] = cookie;
>> }
>>
>> then fetch the desc from the shadow buf cookie in all code path, such
>> as mvneta_rx() etc.
>>
>> Both solutions should not have the problems pointed out by Arnd.
>
> Wait, since you compute an index 'i' here, can't you just store 'i'
> directly in the descriptor instead of the pointer?
>
> Arnd
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply
* Re: [net-next PATCH v3] net: dummy: Introduce dummy virtual functions
From: Eric Dumazet @ 2016-11-23 13:26 UTC (permalink / raw)
To: Phil Sutter; +Cc: David Miller, netdev, Sabrina Dubroca
In-Reply-To: <20161123124125.3374-1-phil@nwl.cc>
On Wed, 2016-11-23 at 13:41 +0100, Phil Sutter wrote:
> +struct vf_data_storage {
> + unsigned char vf_mac[ETH_ALEN];
> + u16 pf_vlan; /* When set, guest VLAN config not allowed. */
> + u16 pf_qos;
> + __be16 vlan_proto;
> + u16 min_tx_rate;
> + u16 max_tx_rate;
> + u8 spoofchk_enabled;
> + bool rss_query_enabled;
> + u8 trusted;
> + int link_state;
> +};
Could you use proper indentation of field names ?
struct vf_data_storage {
u8 vf_mac[ETH_ALEN];
u16 pf_vlan; /* When set, guest VLAN config not allowed. */
u16 pf_qos;
__be16 vlan_proto;
u16 min_tx_rate;
u16 max_tx_rate;
u8 spoofchk_enabled;
bool rss_query_enabled;
u8 trusted;
int link_state;
};
struct dummy_priv {
int num_vfs;
struct vf_data_storage *vfinfo;
};
Thanks.
^ permalink raw reply
* [PATCH] can: bcm: fix support for CAN FD frames
From: Oliver Hartkopp @ 2016-11-23 13:33 UTC (permalink / raw)
To: linux-can, Andrey Konovalov, netdev; +Cc: Oliver Hartkopp
Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.
As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.
This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.
Reference: http://marc.info/?l=linux-netdev&m=147980658909653
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
---
net/can/bcm.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25..436a753 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
(CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
(CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
u32 count;
u32 nframes;
u32 currframe;
- struct canfd_frame *frames;
- struct canfd_frame *last_frames;
+ /* void pointers to arrays of struct can[fd]_frame */
+ void *frames;
+ void *last_frames;
struct canfd_frame sframe;
struct canfd_frame last_sframe;
struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
if (op->flags & RX_FILTER_ID) {
/* the easiest case */
- bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+ bcm_rx_update_and_send(op, op->last_frames, rxframe);
goto rx_starttimer;
}
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (msg_head->nframes) {
/* update CAN frames content */
- err = memcpy_from_msg((u8 *)op->frames, msg,
+ err = memcpy_from_msg(op->frames, msg,
msg_head->nframes * op->cfsiz);
if (err < 0)
return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
}
if (msg_head->nframes) {
- err = memcpy_from_msg((u8 *)op->frames, msg,
+ err = memcpy_from_msg(op->frames, msg,
msg_head->nframes * op->cfsiz);
if (err < 0) {
if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* check flags */
if (op->flags & RX_RTR_FRAME) {
+ struct canfd_frame *frame0 = op->frames;
/* no timers in RTR-mode */
hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
* prevent a full-load-loopback-test ... ;-]
*/
if ((op->flags & TX_CP_CAN_ID) ||
- (op->frames[0].can_id == op->can_id))
- op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+ (frame0->can_id == op->can_id))
+ frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
} else {
if (op->flags & SETTIMER) {
--
2.10.2
^ permalink raw reply related
* Re: [RFC PATCH v2 1/2] macb: Add 1588 support in Cadence GEM.
From: Andrei Pistirica @ 2016-11-23 13:34 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam, punnaia, michals, anirudh,
boris.brezillon, alexandre.belloni, tbultel
In-Reply-To: <20161120191823.GA6950@localhost.localdomain>
On 20.11.2016 20:18, Richard Cochran wrote:
> On Fri, Nov 18, 2016 at 03:21:51PM +0100, Andrei Pistirica wrote:
>> - Frequency adjustment is not directly supported by this IP.
>
> This statement still makes no sense. Doesn't the following text...
This statement is inherited from original patch, and it refers to the
fact that it doesn't change directly the frequency, it changes the
increment value.
I'll remove it to avoid any confusion.
>
>> addend is the initial value ns increment and similarly addendesub.
>> The ppb (parts per billion) provided is used as
>> ns_incr = addend +/- (ppb/rate).
>> Similarly the remainder of the above is used to populate subns increment.
>
> describe how frequency adjustment is in fact supported?
Ack, I will clarify.
>
>> +config MACB_USE_HWSTAMP
>> + bool "Use IEEE 1588 hwstamp"
>> + depends on MACB
>> + default y
>> + select PTP_1588_CLOCK
>
> This "select" pattern is going to be replaced with "imply" soon.
>
> http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1269181.html
>
> You should use the new "imply" key word and reference that series in
> your change log.
Ack.
>
>> diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
>> index 3f385ab..2ee9af8 100644
>> --- a/drivers/net/ethernet/cadence/macb.h
>> +++ b/drivers/net/ethernet/cadence/macb.h
>> @@ -10,6 +10,10 @@
>> #ifndef _MACB_H
>> #define _MACB_H
>>
>> +#include <linux/net_tstamp.h>
>> +#include <linux/ptp_clock.h>
>> +#include <linux/ptp_clock_kernel.h>
>
> Don't need net_tstamp.h here. Move it into the .c file please.
Ack.
>
>> @@ -840,8 +902,26 @@ struct macb {
>>
>> unsigned int rx_frm_len_mask;
>> unsigned int jumbo_max_len;
>> +
>> +#ifdef CONFIG_MACB_USE_HWSTAMP
>> + unsigned int hwts_tx_en;
>> + unsigned int hwts_rx_en;
>
> These two can be bool'eans.
Ack.
>
>> + spinlock_t tsu_clk_lock;
>> + unsigned int tsu_rate;
>> +
>> + struct ptp_clock *ptp_clock;
>> + struct ptp_clock_info ptp_caps;
>> + unsigned int ns_incr;
>> + unsigned int subns_incr;
>
> These two are 32 bit register values, right? Then use the u32 type.
Yes. I will make the change.
>
>> +#endif
>> };
>
>> +static inline void macb_tsu_set_time(struct macb *bp,
>> + const struct timespec64 *ts)
>> +{
>> + u32 ns, sech, secl;
>> + s64 word_mask = 0xffffffff;
>> +
>> + sech = (u32)ts->tv_sec;
>> + secl = (u32)ts->tv_sec;
>> + ns = ts->tv_nsec;
>> + if (ts->tv_sec > word_mask)
>> + sech = (ts->tv_sec >> 32);
>> +
>> + spin_lock(&bp->tsu_clk_lock);
>> +
>> + /* TSH doesn't latch the time and no atomicity! */
>> + gem_writel(bp, TSH, sech);
>> + gem_writel(bp, TSL, secl);
>
> If TN overflows here then the clock will be off by one whole second!
> Why not clear TN first?
Ack.
>
>> + gem_writel(bp, TN, ns);
>> +
>> + spin_unlock(&bp->tsu_clk_lock);
>> +}
>> +
>> +static int macb_ptp_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
>> +{
>> + struct macb *bp = container_of(ptp, struct macb, ptp_caps);
>> + u32 addend, addend_frac, rem;
>> + u64 drift_tmr, diff, diff_frac = 0;
>> + int neg_adj = 0;
>> +
>> + if (ppb < 0) {
>> + neg_adj = 1;
>> + ppb = -ppb;
>> + }
>> +
>> + /* drift/period */
>> + drift_tmr = (bp->ns_incr * ppb) +
>> + ((bp->subns_incr * ppb) >> 16);
>
> What? Why the 16 bit shift? Last time your said it was 24 bits.
SAMA5D2/3/4 uses GEM-PTP version (16bit), while Harini wrote a driver
for GEM-GXL (24bit). Probably will be a patch on top of this one for
GXL. This confusion was because I tried to keep the original patch
unchanged.
>
>> + /* drift/cycle */
>> + diff = div_u64_rem(drift_tmr, 1000000000ULL, &rem);
>> +
>> + /* drift fraction/cycle, if necessary */
>> + if (rem) {
>> + u64 fraction = rem;
>> + fraction = fraction << 16;
>> +
>> + diff_frac = div_u64(fraction, 1000000000ULL);
>
> If you use a combined tuning word like I explained last time, then you
> will not need a second division.
From what I understand, your suggestion is:
(ns | frac) * ppb = (total_ns | total_frac)
(total_ns | total_frac) / 10^9 = (adj_ns | adj_frac)
This is correct iff total_ns/10^9 >= 1, but the problem is that there
are missed fractions due to the following approximation:
frac*ppb =~
(ns*ppb+frac*ppb*2^16)*2^16-10^9*2^16*flor(ns*ppb+frac*ppb*2^16, 10^9).
An example which uses values from a real test:
let ppb=4891, ns=12 and frac=3158
- using suggested algorithm, yields: adj_ns = 0 and adj_frac = 0
- using in-place algorithm, yields: adj_ns = 0, adj_frac = 4
You can check the calculus.
Therefore, I would like to keep the in-place algorithm.
>
> Also, please use the new adjfine() PHC method, as adjfreq() is now deprecated.
Yes, I will port the patches on net-next and use adjfine.
>
>> + }
>> +
>> + /* adjustmets */
>> + addend = neg_adj ? (bp->ns_incr - diff) : (bp->ns_incr + diff);
>> + addend_frac = neg_adj ? (bp->subns_incr - diff_frac) :
>> + (bp->subns_incr + diff_frac);
>> +
>> + spin_lock(&bp->tsu_clk_lock);
>> +
>> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, addend_frac));
>> + gem_writel(bp, TI, GEM_BF(NSINCR, addend));
>> +
>> + spin_unlock(&bp->tsu_clk_lock);
>> + return 0;
>> +}
>
>> +void macb_ptp_init(struct net_device *ndev)
>> +{
>> + struct macb *bp = netdev_priv(ndev);
>> + struct timespec64 now;
>> + u32 rem = 0;
>> +
>> + if (!(bp->caps | MACB_CAPS_GEM_HAS_PTP)){
>> + netdev_vdbg(bp->dev, "Platform does not support PTP!\n");
>> + return;
>> + }
>> +
>> + spin_lock_init(&bp->tsu_clk_lock);
>> +
>> + bp->ptp_caps = macb_ptp_caps;
>> + bp->tsu_rate = clk_get_rate(bp->pclk);
>> +
>> + getnstimeofday64(&now);
>> + macb_tsu_set_time(bp, (const struct timespec64 *)&now);
>> +
>> + bp->ns_incr = div_u64_rem(NSEC_PER_SEC, bp->tsu_rate, &rem);
>> + if (rem) {
>> + u64 adj = rem;
>> + /* Multiply by 2^16 as subns register is 16 bits */
>
> Last time you said:
>> + /* Multiple by 2^24 as subns field is 24 bits */
>
>> + adj <<= 16;
>> + bp->subns_incr = div_u64(adj, bp->tsu_rate);
>> + } else {
>> + bp->subns_incr = 0;
>> + }
>> +
>> + gem_writel(bp, TISUBN, GEM_BF(SUBNSINCR, bp->subns_incr));
>> + gem_writel(bp, TI, GEM_BF(NSINCR, bp->ns_incr));
>> + gem_writel(bp, TA, 0);
>> +
>> + bp->ptp_clock = ptp_clock_register(&bp->ptp_caps, NULL);
>
> You call regsiter, but you never call unregister!
Ack. I did a stupid mistake... sorry.
>
>> + if (IS_ERR(&bp->ptp_clock)) {
>> + bp->ptp_clock = NULL;
>> + pr_err("ptp clock register failed\n");
>> + return;
>> + }
>> +
>> + dev_info(&bp->pdev->dev, "%s ptp clock registered.\n", GMAC_TIMER_NAME);
>> +}
>> +
>> --
>> 1.9.1
>>
>
> Thanks,
> Richard
>
Regards,
Andrei
^ permalink raw reply
* Re: [RFC PATCH v2 1/2] macb: Add 1588 support in Cadence GEM.
From: Andrei Pistirica @ 2016-11-23 13:36 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam, punnaia, michals, anirudh,
boris.brezillon, alexandre.belloni, tbultel
In-Reply-To: <20161120193720.GA7874@localhost.localdomain>
On 20.11.2016 20:37, Richard Cochran wrote:
> On Fri, Nov 18, 2016 at 03:21:51PM +0100, Andrei Pistirica wrote:
>> +#ifdef CONFIG_MACB_USE_HWSTAMP
>> +void macb_ptp_init(struct net_device *ndev);
>> +#else
>> +void macb_ptp_init(struct net_device *ndev) { }
>
> static inline ^^^
I can do static inline only when PTP is not enabled (on else branch),
thus the empty function is defined in the header file (since the init
function is defined in macb_ptp and used in macb). To differentiate
between macb versions, I'll add a wrapper.
Would this be ok?
>
>> +#endif
>
>
>> +void macb_ptp_init(struct net_device *ndev)
>> +{
>> + struct macb *bp = netdev_priv(ndev);
>> + struct timespec64 now;
>> + u32 rem = 0;
>> +
>> + if (!(bp->caps | MACB_CAPS_GEM_HAS_PTP)){
>> + netdev_vdbg(bp->dev, "Platform does not support PTP!\n");
>> + return;
>> + }
>
> You would have needed '&' and not '|' here.
Yes. Another stupid mistake... sorry. I will be more careful next time.
>
> Also, using a flag limits the code to your platform. This works for
> you, but it is short sighted. The other MACB PTP blocks have
> different register layouts, and this patch does not lay the ground
> work for the others.
>
> The driver needs to be designed to support the other platforms.
It will support Xilinx.
>
> Thanks,
> Richard
>
Regards,
Andrei
^ permalink raw reply
* Re: [RFC PATCH v2 2/2] macb: Enable 1588 support in SAMA5D2 platform.
From: Andrei Pistirica @ 2016-11-23 13:35 UTC (permalink / raw)
To: Richard Cochran
Cc: netdev, linux-kernel, linux-arm-kernel, davem, nicolas.ferre,
harinikatakamlinux, harini.katakam, punnaia, michals, anirudh,
boris.brezillon, alexandre.belloni, tbultel
In-Reply-To: <20161120195413.GB7874@localhost.localdomain>
On 20.11.2016 20:54, Richard Cochran wrote:
> On Fri, Nov 18, 2016 at 03:21:52PM +0100, Andrei Pistirica wrote:
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index d975882..eb66b76 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -697,6 +697,8 @@ static void macb_tx_interrupt(struct macb_queue *queue)
>>
>> /* First, update TX stats if needed */
>> if (skb) {
>> + macb_ptp_do_txstamp(bp, skb);
>
> This is in the hot path, and so you should have an inline wrapper that
> tests (bp->hwts_tx_en) and THEN calls into macb_ptp.c
Ack.
>
> In case PTP isn't configured, then the inline wrapper should be empty.
>
>> netdev_vdbg(bp->dev, "skb %u (data %p) TX complete\n",
>> macb_tx_ring_wrap(tail), skb->data);
>> bp->stats.tx_packets++;
>> @@ -853,6 +855,8 @@ static int gem_rx(struct macb *bp, int budget)
>> GEM_BFEXT(RX_CSUM, ctrl) & GEM_RX_CSUM_CHECKED_MASK)
>> skb->ip_summed = CHECKSUM_UNNECESSARY;
>>
>> + macb_ptp_do_rxstamp(bp, skb);
>
> Same here.
>
>> bp->stats.rx_packets++;
>> bp->stats.rx_bytes += skb->len;
>>
>> @@ -1946,6 +1950,8 @@ static int macb_open(struct net_device *dev)
>>
>> netif_tx_start_all_queues(dev);
>>
>> + macb_ptp_init(dev);
>
> This leaks PHC instances starting the second time that the interface goes up!
Yes, I will call unregister at interface down.
>
>> return 0;
>> }
>>
>> @@ -2204,7 +2210,7 @@ static const struct ethtool_ops gem_ethtool_ops = {
>> .get_regs_len = macb_get_regs_len,
>> .get_regs = macb_get_regs,
>> .get_link = ethtool_op_get_link,
>> - .get_ts_info = ethtool_op_get_ts_info,
>> + .get_ts_info = macb_get_ts_info,
>
> You enable the time stamping logic unconditionally here ...
I will add a wrapper to test if macb is gem and if it has PTP
capability, otherwise call ethtool_op_get_ts_info.
>
>> .get_ethtool_stats = gem_get_ethtool_stats,
>> .get_strings = gem_get_ethtool_strings,
>> .get_sset_count = gem_get_sset_count,
>> @@ -2221,7 +2227,14 @@ static int macb_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
>> if (!phydev)
>> return -ENODEV;
>>
>> - return phy_mii_ioctl(phydev, rq, cmd);
>> + switch (cmd) {
>> + case SIOCSHWTSTAMP:
>> + return macb_hwtst_set(dev, rq, cmd);
>> + case SIOCGHWTSTAMP:
>> + return macb_hwtst_get(dev, rq);
>
> and here.
>
> Is that logic always available on every MACB device? If so, is the
> implementation also identical?
As before, I will add a wrapper and the related tests.
>
> Thanks,
> Richard
>
Regards,
Andrei
^ permalink raw reply
* Re: [PATCH net 1/2] r8152: fix the sw rx checksum is unavailable
From: Mark Lord @ 2016-11-23 13:41 UTC (permalink / raw)
To: Hayes Wang, netdev@vger.kernel.org
Cc: nic_swsd, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
In-Reply-To: <0835B3720019904CB8F7AA43166CEEB2010517CE@RTITMBSV03.realtek.com.tw>
What does this code do:
>static void r8153_set_rx_early_size(struct r8152 *tp)
>{
> u32 mtu = tp->netdev->mtu;
> u32 ocp_data = (agg_buf_sz - mtu - VLAN_ETH_HLEN - VLAN_HLEN) / 4;
>
> ocp_write_word(tp, MCU_TYPE_USB, USB_RX_EARLY_SIZE, ocp_data);
>}
How is ocp_data used by the hardware?
Shouldn't the calculation also include sizeof(rx_desc) in there somewhere?
Thanks
--
Mark Lord
Real-Time Remedies Inc.
mlord@pobox.com
^ permalink raw reply
* [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
From: Geert Uytterhoeven @ 2016-11-23 13:44 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde, Ramesh Shanmugasundaram,
Chris Paterson
Cc: linux-can, netdev, devicetree, linux-renesas-soc,
Geert Uytterhoeven
According to both DTS (example and actual files), and Linux driver code,
the first interrupt specifier should be the Channel interrupt, while the
second interrupt specifier should be the Global interrupt.
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
index 22a6f10bab057f46..58c27cd839e3a2ac 100644
--- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
+++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
@@ -11,7 +11,7 @@ Required properties:
family-specific and/or generic versions.
- reg: physical base address and size of the R-Car CAN FD register map.
-- interrupts: interrupt specifier for the Global & Channel interrupts
+- interrupts: interrupt specifiers for the Channel & Global interrupts
- clocks: phandles and clock specifiers for 3 clock inputs.
- clock-names: 3 clock input name strings: "fck", "canfd", "can_clk".
- pinctrl-0: pin control group to be used for this controller.
--
1.9.1
^ permalink raw reply related
* Re: [RFC net-next 0/3] net: bridge: Allow CPU port configuration
From: Ido Schimmel @ 2016-11-23 13:48 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, davem, bridge, stephen, vivien.didelot, andrew, jiri,
idosch
In-Reply-To: <e9bbd22e-215f-a1fd-62ba-c0d33b255154@gmail.com>
Hi Florian,
On Tue, Nov 22, 2016 at 09:56:30AM -0800, Florian Fainelli wrote:
> On 11/22/2016 09:41 AM, Ido Schimmel wrote:
> > Hi Florian,
> >
> > On Mon, Nov 21, 2016 at 11:09:22AM -0800, Florian Fainelli wrote:
> >> Hi all,
> >>
> >> This patch series allows using the bridge master interface to configure
> >> an Ethernet switch port's CPU/management port with different VLAN attributes than
> >> those of the bridge downstream ports/members.
> >>
> >> Jiri, Ido, Andrew, Vivien, please review the impact on mlxsw and mv88e6xxx, I
> >> tested this with b53 and a mockup DSA driver.
> >
> > We'll need to add a check in mlxsw and ignore any VLAN configuration for
> > the bridge device itself. Otherwise, any configuration done on br0 will
> > be propagated to all of its slaves, which is incorrect.
> >
> >>
> >> Open questions:
> >>
> >> - if we have more than one bridge on top of a physical switch, the driver
> >> should keep track of that and verify that we are not going to change
> >> the CPU port VLAN attributes in a way that results in incompatible settings
> >> to be applied
> >>
> >> - if the default behavior is to have all VLANs associated with the CPU port
> >> be ingressing/egressing tagged to the CPU, is this really useful?
> >
> > First of all, I want to be sure that when we say "CPU port", we're
> > talking about the same thing. In mlxsw, the CPU port is a pipe between
> > the device and the host, through which all packets trapped to the host
> > go through. So, when a packet is trapped, the driver reads its Rx
> > descriptor, checks through which port it ingressed, resolves its netdev,
> > sets skb->dev accordingly and injects it to the Rx path via
> > netif_receive_skb(). The CPU port itself isn't represented using a
> > netdev.
>
> In the case of DSA, the CPU port is a normal Ethernet MAC driver, but in
> premise, this driver plus the DSA tag protocol hook do exactly the same
> things as you just describe.
Thanks for the detailed explanation! I also took the time to read
dsa.txt, however I still don't quite understand the motivation for
VLAN filtering on the CPU port. In which cases would you like to prevent
packets from going to the host due to their VLAN header? This change
would make sense to me if you only had a limited number of VLANs you
could enable on the CPU port, but from your response I understand that
this isn't the case.
FWIW, I don't have a problem with patches if they are useful for you,
I'm just trying to understand the use case. We can easily patch mlxsw to
ignore calls with orig_dev=br0.
Thanks!
^ permalink raw reply
* RE: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
From: Ramesh Shanmugasundaram @ 2016-11-23 14:04 UTC (permalink / raw)
To: Geert Uytterhoeven, Wolfgang Grandegger, Marc Kleine-Budde,
Chris Paterson
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
devicetree@vger.kernel.org, linux-renesas-soc@vger.kernel.org
In-Reply-To: <1479908686-14028-1-git-send-email-geert+renesas@glider.be>
Hi Geert,
> Subject: [PATCH] can: rcar_canfd: Correct order of interrupt specifiers
>
> According to both DTS (example and actual files), and Linux driver code,
> the first interrupt specifier should be the Channel interrupt, while the
> second interrupt specifier should be the Global interrupt.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Documentation/devicetree/bindings/net/can/rcar_canfd.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> index 22a6f10bab057f46..58c27cd839e3a2ac 100644
> --- a/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> +++ b/Documentation/devicetree/bindings/net/can/rcar_canfd.txt
> @@ -11,7 +11,7 @@ Required properties:
> family-specific and/or generic versions.
>
> - reg: physical base address and size of the R-Car CAN FD register map.
> -- interrupts: interrupt specifier for the Global & Channel interrupts
> +- interrupts: interrupt specifiers for the Channel & Global interrupts
Thanks for correcting it.
Acked-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Thanks,
Ramesh
^ permalink raw reply
* Re: [PATCH] can: bcm: fix support for CAN FD frames
From: Andrey Konovalov @ 2016-11-23 14:15 UTC (permalink / raw)
To: Oliver Hartkopp; +Cc: linux-can, netdev
In-Reply-To: <20161123133325.1812-1-socketcan@hartkopp.net>
On Wed, Nov 23, 2016 at 2:33 PM, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
> CAN broadcast manager supports CAN and CAN FD data frames.
>
> As these data frames are embedded in struct can[fd]_frames which have a
> different length the access to the provided array of CAN frames became
> dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
> CAN frames the new offset calculation based on op->cfsiz was accidently applied
> to CAN FD frame element lengths.
>
> This fix makes the pointer to the arrays of the different CAN frame types a
> void pointer so that the offset calculation in bytes accesses the correct CAN
> frame elements.
>
> Reference: http://marc.info/?l=linux-netdev&m=147980658909653
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> net/can/bcm.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/net/can/bcm.c b/net/can/bcm.c
> index 8af9d25..436a753 100644
> --- a/net/can/bcm.c
> +++ b/net/can/bcm.c
> @@ -77,7 +77,7 @@
> (CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
> (CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
>
> -#define CAN_BCM_VERSION "20160617"
> +#define CAN_BCM_VERSION "20161123"
>
> MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
> MODULE_LICENSE("Dual BSD/GPL");
> @@ -109,8 +109,9 @@ struct bcm_op {
> u32 count;
> u32 nframes;
> u32 currframe;
> - struct canfd_frame *frames;
> - struct canfd_frame *last_frames;
> + /* void pointers to arrays of struct can[fd]_frame */
> + void *frames;
> + void *last_frames;
> struct canfd_frame sframe;
> struct canfd_frame last_sframe;
> struct sock *sk;
> @@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
>
> if (op->flags & RX_FILTER_ID) {
> /* the easiest case */
> - bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
> + bcm_rx_update_and_send(op, op->last_frames, rxframe);
> goto rx_starttimer;
> }
>
> @@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
>
> if (msg_head->nframes) {
> /* update CAN frames content */
> - err = memcpy_from_msg((u8 *)op->frames, msg,
> + err = memcpy_from_msg(op->frames, msg,
> msg_head->nframes * op->cfsiz);
> if (err < 0)
> return err;
> @@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> }
>
> if (msg_head->nframes) {
> - err = memcpy_from_msg((u8 *)op->frames, msg,
> + err = memcpy_from_msg(op->frames, msg,
> msg_head->nframes * op->cfsiz);
> if (err < 0) {
> if (op->frames != &op->sframe)
> @@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> /* check flags */
>
> if (op->flags & RX_RTR_FRAME) {
> + struct canfd_frame *frame0 = op->frames;
>
> /* no timers in RTR-mode */
> hrtimer_cancel(&op->thrtimer);
> @@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
> * prevent a full-load-loopback-test ... ;-]
> */
> if ((op->flags & TX_CP_CAN_ID) ||
> - (op->frames[0].can_id == op->can_id))
> - op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
> + (frame0->can_id == op->can_id))
> + frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
>
> } else {
> if (op->flags & SETTIMER) {
> --
> 2.10.2
>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
^ permalink raw reply
* [PATCH 1/4] bindings: net: stmmac: correct note about TSO
From: Niklas Cassel @ 2016-11-23 14:24 UTC (permalink / raw)
To: Rob Herring, Mark Rutland, David S. Miller, Giuseppe CAVALLARO,
Alexandre TORGUE, Phil Reid, Niklas Cassel, Eric Engestrom
Cc: Niklas Cassel, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
From: Niklas Cassel <niklas.cassel-VrBV9hrLPhE@public.gmane.org>
snps,tso was previously placed under AXI BUS Mode parameters,
suggesting that the property should be in the stmmac-axi-config node.
TSO (TCP Segmentation Offloading) has nothing to do with AXI BUS Mode
parameters, and the parser actually expects it to be in the root node,
not in the stmmac-axi-config.
Also added a note about snps,tso only being available on GMAC4 and newer.
Signed-off-by: Niklas Cassel <niklas.cassel-VrBV9hrLPhE@public.gmane.org>
---
Documentation/devicetree/bindings/net/stmmac.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/stmmac.txt b/Documentation/devicetree/bindings/net/stmmac.txt
index 41b49e6075f5..b95ff998ba73 100644
--- a/Documentation/devicetree/bindings/net/stmmac.txt
+++ b/Documentation/devicetree/bindings/net/stmmac.txt
@@ -1,7 +1,7 @@
* STMicroelectronics 10/100/1000 Ethernet driver (GMAC)
Required properties:
-- compatible: Should be "snps,dwmac-<ip_version>" "snps,dwmac"
+- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
- reg: Address and length of the register set for the device
- interrupt-parent: Should be the phandle for the interrupt controller
@@ -50,6 +50,8 @@ Optional properties:
- snps,ps-speed: port selection speed that can be passed to the core when
PCS is supported. For example, this is used in case of SGMII
and MAC2MAC connection.
+- snps,tso: this enables the TSO feature otherwise it will be managed by
+ MAC HW capability register. Only for GMAC4 and newer.
- AXI BUS Mode parameters: below the list of all the parameters to program the
AXI register inside the DMA module:
- snps,lpi_en: enable Low Power Interface
@@ -62,8 +64,6 @@ Optional properties:
- snps,fb: fixed-burst
- snps,mb: mixed-burst
- snps,rb: rebuild INCRx Burst
- - snps,tso: this enables the TSO feature otherwise it will be managed by
- MAC HW capability register.
- mdio: with compatible = "snps,dwmac-mdio", create and register mdio bus.
Examples:
--
2.1.4
--
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 related
* [PATCH] dwc_eth_qos: drop duplicate headers
From: Geliang Tang @ 2016-11-23 14:24 UTC (permalink / raw)
To: Lars Persson; +Cc: Geliang Tang, netdev, linux-kernel
Drop duplicate headers types.h and delay.h from dwc_eth_qos.c.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
drivers/net/ethernet/synopsys/dwc_eth_qos.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/ethernet/synopsys/dwc_eth_qos.c b/drivers/net/ethernet/synopsys/dwc_eth_qos.c
index 7053301..acce385 100644
--- a/drivers/net/ethernet/synopsys/dwc_eth_qos.c
+++ b/drivers/net/ethernet/synopsys/dwc_eth_qos.c
@@ -33,7 +33,6 @@
#include <linux/stat.h>
#include <linux/types.h>
-#include <linux/types.h>
#include <linux/slab.h>
#include <linux/delay.h>
#include <linux/mm.h>
@@ -43,7 +42,6 @@
#include <linux/phy.h>
#include <linux/mii.h>
-#include <linux/delay.h>
#include <linux/dma-mapping.h>
#include <linux/vmalloc.h>
--
2.9.3
^ permalink raw reply related
* [PATCH 3/4] net: stmmac: dwmac-generic: add missing compatible strings
From: Niklas Cassel @ 2016-11-23 14:25 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue; +Cc: Niklas Cassel, netdev, linux-kernel
From: Niklas Cassel <niklas.cassel@axis.com>
devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
Since dwmac-generic.c calls stmmac_probe_config_dt explicitly,
another alternative would have been to remove all compatible strings
other than "snps,dwmac" and "st,spear600-gmac" from dwmac-generic.c.
However, that would probably do more good than harm, since when trying
to figure out what hardware a certain driver supports, you usually look
at the compatible strings in the struct of_device_id, and not in some
function defined in a completely different file.
No functional change intended.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
index b1e5f24708c9..52cd365b8e5e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-generic.c
@@ -58,9 +58,12 @@ static int dwmac_generic_probe(struct platform_device *pdev)
static const struct of_device_id dwmac_generic_match[] = {
{ .compatible = "st,spear600-gmac"},
+ { .compatible = "snps,dwmac-3.50a"},
{ .compatible = "snps,dwmac-3.610"},
{ .compatible = "snps,dwmac-3.70a"},
{ .compatible = "snps,dwmac-3.710"},
+ { .compatible = "snps,dwmac-4.00"},
+ { .compatible = "snps,dwmac-4.10a"},
{ .compatible = "snps,dwmac"},
{ }
};
--
2.1.4
^ permalink raw reply related
* [PATCH 4/4] net: stmmac: stmmac_platform: use correct setup function for gmac4
From: Niklas Cassel @ 2016-11-23 14:25 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue; +Cc: Niklas Cassel, netdev, linux-kernel
From: Niklas Cassel <niklas.cassel@axis.com>
devicetree binding for stmmac states:
- compatible: Should be "snps,dwmac-<ip_version>", "snps,dwmac"
For backwards compatibility: "st,spear600-gmac" is also supported.
Previously, when specifying "snps,dwmac-4.10a", "snps,dwmac" as your
compatible string, plat_stmmacenet_data would have both has_gmac and
has_gmac4 set.
This would lead to stmmac_hw_init calling dwmac1000_setup rather than
dwmac4_setup, resulting in a non-functional driver.
This happened since the check for has_gmac is done before the check for
has_gmac4. However, the order should not matter, so it does not make sense
to have both set.
If something is valid for both, you should do as the stmmac_interrupt does:
if (priv->plat->has_gmac || priv->plat->has_gmac4) ...
The places where it was obvious that the author actually meant
if (has_gmac || has_gmac4) rather than if (has_gmac) has been updated.
Signed-off-by: Niklas Cassel <niklas.cassel@axis.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 4 ++--
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
index d5a8122b6033..dd5b38e4cd1f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_ethtool.c
@@ -263,7 +263,7 @@ static void stmmac_ethtool_getdrvinfo(struct net_device *dev,
{
struct stmmac_priv *priv = netdev_priv(dev);
- if (priv->plat->has_gmac)
+ if (priv->plat->has_gmac || priv->plat->has_gmac4)
strlcpy(info->driver, GMAC_ETHTOOL_NAME, sizeof(info->driver));
else
strlcpy(info->driver, MAC100_ETHTOOL_NAME,
@@ -448,7 +448,7 @@ static void stmmac_ethtool_gregs(struct net_device *dev,
memset(reg_space, 0x0, REG_SPACE_SIZE);
- if (!priv->plat->has_gmac) {
+ if (!(priv->plat->has_gmac || priv->plat->has_gmac4)) {
/* MAC registers */
for (i = 0; i < 12; i++)
reg_space[i] = readl(priv->ioaddr + (i * 4));
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 4d544c34c1f2..c8a59f396c6e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -291,6 +291,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, const char **mac)
if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
of_device_is_compatible(np, "snps,dwmac-4.10a")) {
plat->has_gmac4 = 1;
+ plat->has_gmac = 0;
plat->pmt = 1;
plat->tso_en = of_property_read_bool(np, "snps,tso");
}
--
2.1.4
^ permalink raw reply related
* Re: [PATCH] cpsw: ethtool: add support for getting/setting EEE registers
From: Rami Rosen @ 2016-11-23 14:31 UTC (permalink / raw)
To: yegorslists
Cc: Netdev, Linux OMAP Mailing List, grygorii.strashko, mugunthanvnm
In-Reply-To: <1479897248-5923-1-git-send-email-yegorslists@googlemail.com>
Hi, Yegor,
Minor comment: these methods should be static.
+int cpsw_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
+{
...
...
+int cpsw_set_eee(struct net_device *ndev, struct ethtool_eee *edata)
...
Regards,
Rami Rosen
^ permalink raw reply
* [PATCH] can: bcm: fix support for CAN FD frames
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, linux-can, kernel, Oliver Hartkopp, linux-stable,
Marc Kleine-Budde
In-Reply-To: <20161123143430.24985-1-mkl@pengutronix.de>
From: Oliver Hartkopp <socketcan@hartkopp.net>
Since commit 6f3b911d5f29b98 ("can: bcm: add support for CAN FD frames") the
CAN broadcast manager supports CAN and CAN FD data frames.
As these data frames are embedded in struct can[fd]_frames which have a
different length the access to the provided array of CAN frames became
dependend of op->cfsiz. By using a struct canfd_frame pointer for the array of
CAN frames the new offset calculation based on op->cfsiz was accidently applied
to CAN FD frame element lengths.
This fix makes the pointer to the arrays of the different CAN frame types a
void pointer so that the offset calculation in bytes accesses the correct CAN
frame elements.
Reference: http://marc.info/?l=linux-netdev&m=147980658909653
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/bcm.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/can/bcm.c b/net/can/bcm.c
index 8af9d25ff988..436a7537e6a9 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -77,7 +77,7 @@
(CAN_EFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG) : \
(CAN_SFF_MASK | CAN_EFF_FLAG | CAN_RTR_FLAG))
-#define CAN_BCM_VERSION "20160617"
+#define CAN_BCM_VERSION "20161123"
MODULE_DESCRIPTION("PF_CAN broadcast manager protocol");
MODULE_LICENSE("Dual BSD/GPL");
@@ -109,8 +109,9 @@ struct bcm_op {
u32 count;
u32 nframes;
u32 currframe;
- struct canfd_frame *frames;
- struct canfd_frame *last_frames;
+ /* void pointers to arrays of struct can[fd]_frame */
+ void *frames;
+ void *last_frames;
struct canfd_frame sframe;
struct canfd_frame last_sframe;
struct sock *sk;
@@ -681,7 +682,7 @@ static void bcm_rx_handler(struct sk_buff *skb, void *data)
if (op->flags & RX_FILTER_ID) {
/* the easiest case */
- bcm_rx_update_and_send(op, &op->last_frames[0], rxframe);
+ bcm_rx_update_and_send(op, op->last_frames, rxframe);
goto rx_starttimer;
}
@@ -1068,7 +1069,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
if (msg_head->nframes) {
/* update CAN frames content */
- err = memcpy_from_msg((u8 *)op->frames, msg,
+ err = memcpy_from_msg(op->frames, msg,
msg_head->nframes * op->cfsiz);
if (err < 0)
return err;
@@ -1118,7 +1119,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
}
if (msg_head->nframes) {
- err = memcpy_from_msg((u8 *)op->frames, msg,
+ err = memcpy_from_msg(op->frames, msg,
msg_head->nframes * op->cfsiz);
if (err < 0) {
if (op->frames != &op->sframe)
@@ -1163,6 +1164,7 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
/* check flags */
if (op->flags & RX_RTR_FRAME) {
+ struct canfd_frame *frame0 = op->frames;
/* no timers in RTR-mode */
hrtimer_cancel(&op->thrtimer);
@@ -1174,8 +1176,8 @@ static int bcm_rx_setup(struct bcm_msg_head *msg_head, struct msghdr *msg,
* prevent a full-load-loopback-test ... ;-]
*/
if ((op->flags & TX_CP_CAN_ID) ||
- (op->frames[0].can_id == op->can_id))
- op->frames[0].can_id = op->can_id & ~CAN_RTR_FLAG;
+ (frame0->can_id == op->can_id))
+ frame0->can_id = op->can_id & ~CAN_RTR_FLAG;
} else {
if (op->flags & SETTIMER) {
--
2.10.2
^ permalink raw reply related
* [patch net-next v2 00/11] ipv4: fib: Allow modules to dump FIB tables
From: Jiri Pirko @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz, roopa,
dsa, nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, hannes, kaber
From: Jiri Pirko <jiri@mellanox.com>
Ido says:
In kernel 4.9 the switchdev-specific FIB offload mechanism was replaced
by a new FIB notification chain to which modules could register in order
to be notified about the addition and deletion of FIB entries. The
motivation for this change was that switchdev drivers need to be able to
reflect the entire FIB table and not only FIBs configured on top of the
port netdevs themselves. This is useful in case of in-band management.
The fundamental problem with this approach is that upon registration
listeners lose all the information previously sent in the chain and
thus have an incomplete view of the FIB tables, which can result in
packet loss. This patchset fixes that by introducing a new API to dump
the FIB tables.
The entire dump process is done under RCU and thus the FIB notification
chain is converted to be atomic. The listeners are modified accordingly.
This is done in the first seven patches.
The eighth patch adds a change sequence counter to ensure the integrity
of the FIB dump, which is finally introduced in the following patch. The
last two patches modify current listeners of the FIB notification chain
to invoke the dump during their init.
---
v1->v2:
- Add a sequence counter to ensure the integrity of the FIB dump
(David S. Miller, Hannes Frederic Sowa).
- Protect notifications from re-ordering in listeners by using an
ordered workqueue (Hannes Frederic Sowa).
- Introduce fib_info_hold() (Jiri Pirko).
- Relieve rocker from the need to invoke the FIB dump by registering
to the FIB notification chain prior to ports creation.
Ido Schimmel (11):
ipv4: fib: Export free_fib_info()
ipv4: fib: Add fib_info_hold() helper
mlxsw: core: Create an ordered workqueue for FIB offload
mlxsw: spectrum_router: Implement FIB offload in deferred work
rocker: Create an ordered workqueue for FIB offload
rocker: Implement FIB offload in deferred work
ipv4: fib: Convert FIB notification chain to be atomic
ipv4: fib: Allow for consistent FIB dumping
ipv4: fib: Add an API to request a FIB dump
mlxsw: spectrum_router: Request a dump of FIB tables during init
rocker: Register FIB notifier before creating ports
drivers/net/ethernet/mellanox/mlxsw/core.c | 22 ++++
drivers/net/ethernet/mellanox/mlxsw/core.h | 2 +
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 88 ++++++++++++--
drivers/net/ethernet/rocker/rocker.h | 1 +
drivers/net/ethernet/rocker/rocker_main.c | 78 +++++++++++--
drivers/net/ethernet/rocker/rocker_ofdpa.c | 1 +
include/net/ip_fib.h | 6 +
include/net/netns/ipv4.h | 2 +
net/ipv4/fib_frontend.c | 2 +
net/ipv4/fib_semantics.c | 1 +
net/ipv4/fib_trie.c | 126 ++++++++++++++++++++-
11 files changed, 303 insertions(+), 26 deletions(-)
--
2.7.4
^ permalink raw reply
* pull-request: can 2016-11-23
From: Marc Kleine-Budde @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev; +Cc: davem, linux-can, kernel
Hello David,
this is a pull request for net/master.
The patch by Oliver Hartkopp for the broadcast manager (bcm) fixes the CAN-FD
support, which may cause an out-of-bounds access otherwise.
regards,
Marc
---
The following changes since commit c9b8af1330198ae241cd545e1f040019010d44d9:
flow_dissect: call init_default_flow_dissectors() earlier (2016-11-22 14:44:01 -0500)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.9-20161123
for you to fetch changes up to 5499a6b22e5508b921c447757685b0a5e40a07ed:
can: bcm: fix support for CAN FD frames (2016-11-23 15:22:18 +0100)
----------------------------------------------------------------
linux-can-fixes-for-4.9-20161123
----------------------------------------------------------------
Oliver Hartkopp (1):
can: bcm: fix support for CAN FD frames
net/can/bcm.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
^ permalink raw reply
* [patch net-next v2 01/11] ipv4: fib: Export free_fib_info()
From: Jiri Pirko @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz, roopa,
dsa, nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, hannes, kaber
In-Reply-To: <1479911670-4525-1-git-send-email-jiri@resnulli.us>
From: Ido Schimmel <idosch@mellanox.com>
The FIB notification chain is going to be converted to an atomic chain,
which means switchdev drivers will have to offload FIB entries in
deferred work, as hardware operations entail sleeping.
However, while the work is queued fib info might be freed, so a
reference must be taken. To release the reference (and potentially free
the fib info) fib_info_put() will be called, which in turn calls
free_fib_info().
Export free_fib_info() so that modules will be able to invoke
fib_info_put().
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
net/ipv4/fib_semantics.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 388d3e2..c1bc1e9 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -234,6 +234,7 @@ void free_fib_info(struct fib_info *fi)
#endif
call_rcu(&fi->rcu, free_fib_info_rcu);
}
+EXPORT_SYMBOL_GPL(free_fib_info);
void fib_release_info(struct fib_info *fi)
{
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 02/11] ipv4: fib: Add fib_info_hold() helper
From: Jiri Pirko @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz, roopa,
dsa, nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, hannes, kaber
In-Reply-To: <1479911670-4525-1-git-send-email-jiri@resnulli.us>
From: Ido Schimmel <idosch@mellanox.com>
As explained in the previous commit, modules are going to need to take a
reference on fib info and then drop it using fib_info_put().
Add the fib_info_hold() helper to make the code more readable and also
symmetric with fib_info_put().
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Suggested-by: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/ip_fib.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f390c3b..6c67b93 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -397,6 +397,11 @@ static inline void fib_combine_itag(u32 *itag, const struct fib_result *res)
void free_fib_info(struct fib_info *fi);
+static inline void fib_info_hold(struct fib_info *fi)
+{
+ atomic_inc(&fi->fib_clntref);
+}
+
static inline void fib_info_put(struct fib_info *fi)
{
if (atomic_dec_and_test(&fi->fib_clntref))
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 03/11] mlxsw: core: Create an ordered workqueue for FIB offload
From: Jiri Pirko @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz, roopa,
dsa, nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, hannes, kaber
In-Reply-To: <1479911670-4525-1-git-send-email-jiri@resnulli.us>
From: Ido Schimmel <idosch@mellanox.com>
We're going to start processing FIB entries addition / deletion events
in deferred work. These work items must be processed in the order they
were submitted or otherwise we can have differences between the kernel's
FIB table and the device's.
Solve this by creating an ordered workqueue to which these work items
will be submitted to. Note that we can't simply convert the current
workqueue to be ordered, as EMADs re-transmissions are also processed in
deferred work.
Later on, we can migrate other work items to this workqueue, such as FDB
notification processing and nexthop resolution, since they all take the
same lock anyway.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 22 ++++++++++++++++++++++
drivers/net/ethernet/mellanox/mlxsw/core.h | 2 ++
2 files changed, 24 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index bcd7251..a8d9a9c 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -77,6 +77,7 @@ static const char mlxsw_core_driver_name[] = "mlxsw_core";
static struct dentry *mlxsw_core_dbg_root;
static struct workqueue_struct *mlxsw_wq;
+static struct workqueue_struct *mlxsw_owq;
struct mlxsw_core_pcpu_stats {
u64 trap_rx_packets[MLXSW_TRAP_ID_MAX];
@@ -1857,6 +1858,18 @@ int mlxsw_core_schedule_dw(struct delayed_work *dwork, unsigned long delay)
}
EXPORT_SYMBOL(mlxsw_core_schedule_dw);
+int mlxsw_core_schedule_odw(struct delayed_work *dwork, unsigned long delay)
+{
+ return queue_delayed_work(mlxsw_owq, dwork, delay);
+}
+EXPORT_SYMBOL(mlxsw_core_schedule_odw);
+
+void mlxsw_core_flush_owq(void)
+{
+ flush_workqueue(mlxsw_owq);
+}
+EXPORT_SYMBOL(mlxsw_core_flush_owq);
+
static int __init mlxsw_core_module_init(void)
{
int err;
@@ -1864,6 +1877,12 @@ static int __init mlxsw_core_module_init(void)
mlxsw_wq = alloc_workqueue(mlxsw_core_driver_name, WQ_MEM_RECLAIM, 0);
if (!mlxsw_wq)
return -ENOMEM;
+ mlxsw_owq = alloc_ordered_workqueue("%s_ordered", WQ_MEM_RECLAIM,
+ mlxsw_core_driver_name);
+ if (!mlxsw_owq) {
+ err = -ENOMEM;
+ goto err_alloc_ordered_workqueue;
+ }
mlxsw_core_dbg_root = debugfs_create_dir(mlxsw_core_driver_name, NULL);
if (!mlxsw_core_dbg_root) {
err = -ENOMEM;
@@ -1872,6 +1891,8 @@ static int __init mlxsw_core_module_init(void)
return 0;
err_debugfs_create_dir:
+ destroy_workqueue(mlxsw_owq);
+err_alloc_ordered_workqueue:
destroy_workqueue(mlxsw_wq);
return err;
}
@@ -1879,6 +1900,7 @@ static int __init mlxsw_core_module_init(void)
static void __exit mlxsw_core_module_exit(void)
{
debugfs_remove_recursive(mlxsw_core_dbg_root);
+ destroy_workqueue(mlxsw_owq);
destroy_workqueue(mlxsw_wq);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 3de8955..f676ee9 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -156,6 +156,8 @@ enum devlink_port_type mlxsw_core_port_type_get(struct mlxsw_core *mlxsw_core,
u8 local_port);
int mlxsw_core_schedule_dw(struct delayed_work *dwork, unsigned long delay);
+int mlxsw_core_schedule_odw(struct delayed_work *dwork, unsigned long delay);
+void mlxsw_core_flush_owq(void);
#define MLXSW_CONFIG_PROFILE_SWID_COUNT 8
--
2.7.4
^ permalink raw reply related
* [patch net-next v2 04/11] mlxsw: spectrum_router: Implement FIB offload in deferred work
From: Jiri Pirko @ 2016-11-23 14:34 UTC (permalink / raw)
To: netdev
Cc: davem, idosch, eladr, yotamg, nogahf, arkadis, ogerlitz, roopa,
dsa, nikolay, andy, vivien.didelot, andrew, f.fainelli,
alexander.h.duyck, hannes, kaber
In-Reply-To: <1479911670-4525-1-git-send-email-jiri@resnulli.us>
From: Ido Schimmel <idosch@mellanox.com>
FIB offload is currently done in process context with RTNL held, but
we're about to dump the FIB tables in RCU critical section, so we can no
longer sleep.
Instead, defer the operation to process context using deferred work. Make
sure fib info isn't freed while the work is queued by taking a reference
on it and releasing it after the operation is done.
Deferring the operation is valid because the upper layers always assume
the operation was successful. If it's not, then the driver-specific
abort mechanism is called and all routed traffic is directed to slow
path.
The work items are submitted to an ordered workqueue to prevent a
mismatch between the kernel's FIB table and the device's.
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 72 +++++++++++++++++++---
1 file changed, 62 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 683f045..14bed1d 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -593,6 +593,14 @@ static void mlxsw_sp_router_fib_flush(struct mlxsw_sp *mlxsw_sp);
static void mlxsw_sp_vrs_fini(struct mlxsw_sp *mlxsw_sp)
{
+ /* At this stage we're guaranteed not to have new incoming
+ * FIB notifications and the work queue is free from FIBs
+ * sitting on top of mlxsw netdevs. However, we can still
+ * have other FIBs queued. Flush the queue before flushing
+ * the device's tables. No need for locks, as we're the only
+ * writer.
+ */
+ mlxsw_core_flush_owq();
mlxsw_sp_router_fib_flush(mlxsw_sp);
kfree(mlxsw_sp->router.vrs);
}
@@ -1948,30 +1956,74 @@ static void __mlxsw_sp_router_fini(struct mlxsw_sp *mlxsw_sp)
kfree(mlxsw_sp->rifs);
}
-static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
- unsigned long event, void *ptr)
+struct mlxsw_sp_fib_event_work {
+ struct delayed_work dw;
+ struct fib_entry_notifier_info fen_info;
+ struct mlxsw_sp *mlxsw_sp;
+ unsigned long event;
+};
+
+static void mlxsw_sp_router_fib_event_work(struct work_struct *work)
{
- struct mlxsw_sp *mlxsw_sp = container_of(nb, struct mlxsw_sp, fib_nb);
- struct fib_entry_notifier_info *fen_info = ptr;
+ struct mlxsw_sp_fib_event_work *fib_work =
+ container_of(work, struct mlxsw_sp_fib_event_work, dw.work);
+ struct mlxsw_sp *mlxsw_sp = fib_work->mlxsw_sp;
int err;
- if (!net_eq(fen_info->info.net, &init_net))
- return NOTIFY_DONE;
-
- switch (event) {
+ /* Protect internal structures from changes */
+ rtnl_lock();
+ switch (fib_work->event) {
case FIB_EVENT_ENTRY_ADD:
- err = mlxsw_sp_router_fib4_add(mlxsw_sp, fen_info);
+ err = mlxsw_sp_router_fib4_add(mlxsw_sp, &fib_work->fen_info);
if (err)
mlxsw_sp_router_fib4_abort(mlxsw_sp);
+ fib_info_put(fib_work->fen_info.fi);
break;
case FIB_EVENT_ENTRY_DEL:
- mlxsw_sp_router_fib4_del(mlxsw_sp, fen_info);
+ mlxsw_sp_router_fib4_del(mlxsw_sp, &fib_work->fen_info);
+ fib_info_put(fib_work->fen_info.fi);
break;
case FIB_EVENT_RULE_ADD: /* fall through */
case FIB_EVENT_RULE_DEL:
mlxsw_sp_router_fib4_abort(mlxsw_sp);
break;
}
+ rtnl_unlock();
+ kfree(fib_work);
+}
+
+/* Called with rcu_read_lock() */
+static int mlxsw_sp_router_fib_event(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct mlxsw_sp *mlxsw_sp = container_of(nb, struct mlxsw_sp, fib_nb);
+ struct mlxsw_sp_fib_event_work *fib_work;
+ struct fib_notifier_info *info = ptr;
+
+ if (!net_eq(info->net, &init_net))
+ return NOTIFY_DONE;
+
+ fib_work = kzalloc(sizeof(*fib_work), GFP_ATOMIC);
+ if (WARN_ON(!fib_work))
+ return NOTIFY_BAD;
+
+ INIT_DELAYED_WORK(&fib_work->dw, mlxsw_sp_router_fib_event_work);
+ fib_work->mlxsw_sp = mlxsw_sp;
+ fib_work->event = event;
+
+ switch (event) {
+ case FIB_EVENT_ENTRY_ADD: /* fall through */
+ case FIB_EVENT_ENTRY_DEL:
+ memcpy(&fib_work->fen_info, ptr, sizeof(fib_work->fen_info));
+ /* Take referece on fib_info to prevent it from being
+ * freed while work is queued. Release it afterwards.
+ */
+ fib_info_hold(fib_work->fen_info.fi);
+ break;
+ }
+
+ mlxsw_core_schedule_odw(&fib_work->dw, 0);
+
return NOTIFY_DONE;
}
--
2.7.4
^ permalink raw reply related
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