* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Eric Dumazet @ 2016-05-10 22:44 UTC (permalink / raw)
To: Rik van Riel
Cc: Eric Dumazet, Hannes Frederic Sowa, Paolo Abeni, netdev,
David S. Miller, Jiri Pirko, Daniel Borkmann, Alexei Starovoitov,
Alexander Duyck, Tom Herbert, Peter Zijlstra, Ingo Molnar, LKML
In-Reply-To: <1462917747.23934.102.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, 2016-05-10 at 15:02 -0700, Eric Dumazet wrote:
> On Tue, 2016-05-10 at 14:53 -0700, Eric Dumazet wrote:
> > On Tue, 2016-05-10 at 17:35 -0400, Rik van Riel wrote:
> >
> > > You might need another one of these in invoke_softirq()
> > >
> >
> > Excellent.
> >
> > I gave it a quick try (without your suggestion), and host seems to
> > survive a stress test.
> >
> > Of course we do have to fix these problems :
> >
> > [ 147.781629] NOHZ: local_softirq_pending 48
> > [ 147.785546] NOHZ: local_softirq_pending 48
> > [ 147.788344] NOHZ: local_softirq_pending 48
> > [ 147.788992] NOHZ: local_softirq_pending 48
> > [ 147.790943] NOHZ: local_softirq_pending 48
> > [ 147.791232] NOHZ: local_softirq_pending 24a
> > [ 147.791258] NOHZ: local_softirq_pending 48
> > [ 147.791366] NOHZ: local_softirq_pending 48
> > [ 147.792118] NOHZ: local_softirq_pending 48
> > [ 147.793428] NOHZ: local_softirq_pending 48
>
>
> Well, with your suggestion, these warnings disappear ;)
This is really nice.
Under stress number of context switches is really small.
ksoftirqd and my netserver compete equally to get the cpu cycles (on
CPU0)
lpaa23:~# vmstat 1 10
procs -----------memory---------- ---swap-- -----io---- -system-- ----cpu----
r b swpd free buff cache si so bi bo in cs us sy id wa
2 0 0 260668416 37240 2414428 0 0 21 0 329 349 0 3 96 0
1 0 0 260667904 37240 2414428 0 0 0 12 193126 1050 0 2 98 0
1 0 0 260667904 37240 2414428 0 0 0 0 194354 1056 0 2 98 0
1 0 0 260669104 37240 2414492 0 0 0 0 200897 1095 0 2 98 0
1 0 0 260668592 37240 2414492 0 0 0 0 205731 964 0 2 98 0
1 0 0 260678832 37240 2414492 0 0 0 0 201689 981 0 2 98 0
1 0 0 260678832 37240 2414492 0 0 0 0 204899 742 0 2 98 0
1 0 0 260678320 37240 2414492 0 0 0 0 199148 792 0 3 97 0
1 0 0 260678832 37240 2414492 0 0 0 0 196398 766 0 2 98 0
1 0 0 260678832 37240 2414492 0 0 0 0 201930 858 0 2 98 0
And we can see that ksoftirqd/0 runs for longer periods (~500 usec),
instead of stupid 4 usec before the patch. Less overhead.
lpaa23:~# cat /proc/3/sched
ksoftirqd/0 (3, #threads: 1)
-------------------------------------------------------------------
se.exec_start : 1552401.399526
se.vruntime : 237599.421560
se.sum_exec_runtime : 75432.494199
se.nr_migrations : 0
nr_switches : 144333
nr_voluntary_switches : 143828
nr_involuntary_switches : 505
se.load.weight : 1024
se.avg.load_sum : 10445
se.avg.util_sum : 10445
se.avg.load_avg : 0
se.avg.util_avg : 0
se.avg.last_update_time : 1552401399526
policy : 0
prio : 120
clock-delta : 47
lpaa23:~# echo 75432.494199/144333|bc -l
.52262818758703830724
And yes indeed, user space can progress way faster under flood.
lpaa23:~# nstat >/dev/null;sleep 1;nstat | grep Udp
UdpInDatagrams 186132 0.0
UdpInErrors 735462 0.0
UdpOutDatagrams 10 0.0
UdpRcvbufErrors 735461 0.0
^ permalink raw reply
* Re: [RFC PATCH 0/2] net: threadable napi poll loop
From: Eric Dumazet @ 2016-05-10 22:51 UTC (permalink / raw)
To: Hannes Frederic Sowa
Cc: Eric Dumazet, Paolo Abeni, netdev, David S. Miller, Jiri Pirko,
Daniel Borkmann, Alexei Starovoitov, Alexander Duyck, Tom Herbert,
Peter Zijlstra, Ingo Molnar, Rik van Riel, LKML
In-Reply-To: <94f323a9-515e-4d75-cac8-ef0214f0499e@stressinduktion.org>
On Wed, 2016-05-11 at 00:32 +0200, Hannes Frederic Sowa wrote:
> Not only did we want to present this solely as a bugfix but also as as
> performance enhancements in case of virtio (as you can see in the cover
> letter). Given that a long time ago there was a tendency to remove
> softirqs completely, we thought it might be very interesting, that a
> threaded napi in general seems to be absolutely viable nowadays and
> might offer new features.
Well, you did not fix the bug, you worked around by adding yet another
layer, with another sysctl that admins or programs have to manage.
If you have a special need for virtio, do not hide it behind a 'bug fix'
but add it as a features request.
This ksoftirqd issue is real and a fix looks very reasonable.
Please try this patch, as I had very good success with it.
Thanks.
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 17caf4b63342..22463217e3cf 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -56,6 +56,7 @@ EXPORT_SYMBOL(irq_stat);
static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp;
DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
+DEFINE_PER_CPU(bool, ksoftirqd_scheduled);
const char * const softirq_to_name[NR_SOFTIRQS] = {
"HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "BLOCK_IOPOLL",
@@ -73,8 +74,10 @@ static void wakeup_softirqd(void)
/* Interrupts are disabled: no need to stop preemption */
struct task_struct *tsk = __this_cpu_read(ksoftirqd);
- if (tsk && tsk->state != TASK_RUNNING)
+ if (tsk && tsk->state != TASK_RUNNING) {
+ __this_cpu_write(ksoftirqd_scheduled, true);
wake_up_process(tsk);
+ }
}
/*
@@ -162,7 +165,9 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
*/
preempt_count_sub(cnt - 1);
- if (unlikely(!in_interrupt() && local_softirq_pending())) {
+ if (unlikely(!in_interrupt() &&
+ local_softirq_pending() &&
+ !__this_cpu_read(ksoftirqd_scheduled))) {
/*
* Run softirq if any pending. And do it in its own stack
* as we may be calling this deep in a task call stack already.
@@ -340,6 +345,9 @@ void irq_enter(void)
static inline void invoke_softirq(void)
{
+ if (__this_cpu_read(ksoftirqd_scheduled))
+ return;
+
if (!force_irqthreads) {
#ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
/*
@@ -660,6 +668,8 @@ static void run_ksoftirqd(unsigned int cpu)
* in the task stack here.
*/
__do_softirq();
+ if (!local_softirq_pending())
+ __this_cpu_write(ksoftirqd_scheduled, false);
local_irq_enable();
cond_resched_rcu_qs();
return;
^ permalink raw reply related
* [PATCH net 0/2] bnxt_en: Add workaround to detect bad opaque in rx completion.
From: Michael Chan @ 2016-05-10 23:17 UTC (permalink / raw)
To: davem; +Cc: netdev
2-part workaround for this hardware bug.
Michael Chan (2):
bnxt_en: Add workaround to detect bad opaque in rx completion (part 1)
bnxt_en: Add workaround to detect bad opaque in rx completion (part 2)
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 63 +++++++++++++++++++++++++++++++
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 2 +
2 files changed, 65 insertions(+)
--
1.8.3.1
^ permalink raw reply
* [PATCH net 1/2] bnxt_en: Add workaround to detect bad opaque in rx completion (part 1)
From: Michael Chan @ 2016-05-10 23:17 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1462922280-17851-1-git-send-email-michael.chan@broadcom.com>
There is a rare hardware bug that can cause a bad opaque value in the RX
or TPA completion. When this happens, the hardware may have used the
same buffer twice for 2 rx packets. In addition, the driver will also
crash later using the bad opaque as the index into the ring.
The rx opaque value is predictable and is always monotonically increasing.
The workaround is to keep track of the expected next opaque value and
compare it with the one returned by hardware during RX and TPA start
completions. If they miscompare, we will not process any more RX and
TPA completions and exit NAPI. We will then schedule a workqueue to
reset the function.
This patch adds the logic to keep track of the next rx consumer index.
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 3 +++
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 9d4e8e1..58999cd 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -867,6 +867,7 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
rxr->rx_prod = NEXT_RX(prod);
cons = NEXT_RX(cons);
+ rxr->rx_next_cons = NEXT_RX(cons);
cons_rx_buf = &rxr->rx_buf_ring[cons];
bnxt_reuse_rx_data(rxr, cons, cons_rx_buf->data);
@@ -1245,6 +1246,7 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
next_rx:
rxr->rx_prod = NEXT_RX(prod);
+ rxr->rx_next_cons = NEXT_RX(cons);
next_rx_no_prod:
*raw_cons = tmp_raw_cons;
@@ -2486,6 +2488,7 @@ static void bnxt_clear_ring_indices(struct bnxt *bp)
rxr->rx_prod = 0;
rxr->rx_agg_prod = 0;
rxr->rx_sw_agg_prod = 0;
+ rxr->rx_next_cons = 0;
}
}
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 8b823ff..52f2d74 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -584,6 +584,7 @@ struct bnxt_rx_ring_info {
u16 rx_prod;
u16 rx_agg_prod;
u16 rx_sw_agg_prod;
+ u16 rx_next_cons;
void __iomem *rx_doorbell;
void __iomem *rx_agg_doorbell;
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 2/2] bnxt_en: Add workaround to detect bad opaque in rx completion (part 2)
From: Michael Chan @ 2016-05-10 23:18 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1462922280-17851-1-git-send-email-michael.chan@broadcom.com>
Add detection and recovery code when the hardware returned opaque value
does not match the expected consumer index. Once the issue is detected,
we skip the processing of all RX and LRO/GRO packets. These completion
entries are discarded without sending the SKB to the stack and without
producing new buffers. The function will be reset from a workqueue.
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 60 +++++++++++++++++++++++++++++++
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 1 +
2 files changed, 61 insertions(+)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 58999cd..c39a7f5 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -813,6 +813,46 @@ static inline struct sk_buff *bnxt_copy_skb(struct bnxt_napi *bnapi, u8 *data,
return skb;
}
+static int bnxt_discard_rx(struct bnxt *bp, struct bnxt_napi *bnapi,
+ u32 *raw_cons, void *cmp)
+{
+ struct bnxt_cp_ring_info *cpr = &bnapi->cp_ring;
+ struct rx_cmp *rxcmp = cmp;
+ u32 tmp_raw_cons = *raw_cons;
+ u8 cmp_type, agg_bufs = 0;
+
+ cmp_type = RX_CMP_TYPE(rxcmp);
+
+ if (cmp_type == CMP_TYPE_RX_L2_CMP) {
+ agg_bufs = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) &
+ RX_CMP_AGG_BUFS) >>
+ RX_CMP_AGG_BUFS_SHIFT;
+ } else if (cmp_type == CMP_TYPE_RX_L2_TPA_END_CMP) {
+ struct rx_tpa_end_cmp *tpa_end = cmp;
+
+ agg_bufs = (le32_to_cpu(tpa_end->rx_tpa_end_cmp_misc_v1) &
+ RX_TPA_END_CMP_AGG_BUFS) >>
+ RX_TPA_END_CMP_AGG_BUFS_SHIFT;
+ }
+
+ if (agg_bufs) {
+ if (!bnxt_agg_bufs_valid(bp, cpr, agg_bufs, &tmp_raw_cons))
+ return -EBUSY;
+ }
+ *raw_cons = tmp_raw_cons;
+ return 0;
+}
+
+static void bnxt_sched_reset(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
+{
+ if (!rxr->bnapi->in_reset) {
+ rxr->bnapi->in_reset = true;
+ set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
+ schedule_work(&bp->sp_task);
+ }
+ rxr->rx_next_cons = 0xffff;
+}
+
static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
struct rx_tpa_start_cmp *tpa_start,
struct rx_tpa_start_cmp_ext *tpa_start1)
@@ -830,6 +870,11 @@ static void bnxt_tpa_start(struct bnxt *bp, struct bnxt_rx_ring_info *rxr,
prod_rx_buf = &rxr->rx_buf_ring[prod];
tpa_info = &rxr->rx_tpa[agg_id];
+ if (unlikely(cons != rxr->rx_next_cons)) {
+ bnxt_sched_reset(bp, rxr);
+ return;
+ }
+
prod_rx_buf->data = tpa_info->data;
mapping = tpa_info->mapping;
@@ -981,6 +1026,14 @@ static inline struct sk_buff *bnxt_tpa_end(struct bnxt *bp,
dma_addr_t mapping;
struct sk_buff *skb;
+ if (unlikely(bnapi->in_reset)) {
+ int rc = bnxt_discard_rx(bp, bnapi, raw_cons, tpa_end);
+
+ if (rc < 0)
+ return ERR_PTR(-EBUSY);
+ return NULL;
+ }
+
tpa_info = &rxr->rx_tpa[agg_id];
data = tpa_info->data;
prefetch(data);
@@ -1147,6 +1200,12 @@ static int bnxt_rx_pkt(struct bnxt *bp, struct bnxt_napi *bnapi, u32 *raw_cons,
cons = rxcmp->rx_cmp_opaque;
rx_buf = &rxr->rx_buf_ring[cons];
data = rx_buf->data;
+ if (unlikely(cons != rxr->rx_next_cons)) {
+ int rc1 = bnxt_discard_rx(bp, bnapi, raw_cons, rxcmp);
+
+ bnxt_sched_reset(bp, rxr);
+ return rc1;
+ }
prefetch(data);
agg_bufs = (le32_to_cpu(rxcmp->rx_cmp_misc_v1) & RX_CMP_AGG_BUFS) >>
@@ -4465,6 +4524,7 @@ static void bnxt_enable_napi(struct bnxt *bp)
int i;
for (i = 0; i < bp->cp_nr_rings; i++) {
+ bp->bnapi[i]->in_reset = false;
bnxt_enable_poll(bp->bnapi[i]);
napi_enable(&bp->bnapi[i]->napi);
}
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 52f2d74..de9d53e 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -637,6 +637,7 @@ struct bnxt_napi {
#ifdef CONFIG_NET_RX_BUSY_POLL
atomic_t poll_state;
#endif
+ bool in_reset;
};
#ifdef CONFIG_NET_RX_BUSY_POLL
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver
From: Timur Tabi @ 2016-05-10 23:18 UTC (permalink / raw)
To: Florian Fainelli, netdev, linux-kernel, devicetree, linux-arm-msm,
sdharia, Shanker Donthineni, Greg Kroah-Hartman, vikrams, cov,
gavidov, Rob Herring, andrew, bjorn.andersson, Mark Langsdorf,
Jon Masters, Andy Gross, David S. Miller
In-Reply-To: <571A8207.60704@gmail.com>
Florian Fainelli wrote:
> Are you utilizing the PHYLIB APIs properly? You need at least a
> phy_start() to start the PHY state machine, and an adjust_link callback
> to be provided to phy_connect() (or of_phy_connect()) to manage link
> state changes. And that's the very basic minimum here, there could be
> additional APIs that you may end up using.
>
> There are tons of example in tree of drivers doing this, bcmgenet,
> bcmsysport, tg3 etc.
Thank you. I think I finally got phylib working, more or less.
Unfortunately, it seems I have some kind of race condition. The driver
has a lot that's wrong with it, and I'm trying to fix it all. One crazy
the driver does is it create a workqueue to handle a lot of the tasks
that would normally be handled in the interrupt handler itself.
With phylib support, I know my driver can call phy_mac_interrupt() when
it gets a link status change interrupt. I then have an .adjust_link
callback which starts or stops the mac accordingly.
My problem is that I'm not really sure what adjust_link is supposed to
be doing. In addition, it seems that I need to keep the workqueue
running, otherwise the interface will not function. I bring the
interface up, and the driver reports success, but pings do not work.
I'm getting really frustrated. The sample code isn't really helping a
whole lot, because I lack a fundamental understanding of what needs to
be done. None of the documentation I've read is helpful, and I don't
know how to debug it.
Can you give me some advice on how to debug this?
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Forum, a Linux Foundation collaborative project.
^ permalink raw reply
* Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver
From: Florian Fainelli @ 2016-05-10 23:26 UTC (permalink / raw)
To: Timur Tabi, netdev, linux-kernel, devicetree, linux-arm-msm,
sdharia, Shanker Donthineni, Greg Kroah-Hartman, vikrams, cov,
gavidov, Rob Herring, andrew, bjorn.andersson, Mark Langsdorf,
Jon Masters, Andy Gross, David S. Miller
In-Reply-To: <57326C44.8020906@codeaurora.org>
On 05/10/2016 04:18 PM, Timur Tabi wrote:
> Florian Fainelli wrote:
>> Are you utilizing the PHYLIB APIs properly? You need at least a
>> phy_start() to start the PHY state machine, and an adjust_link callback
>> to be provided to phy_connect() (or of_phy_connect()) to manage link
>> state changes. And that's the very basic minimum here, there could be
>> additional APIs that you may end up using.
>>
>> There are tons of example in tree of drivers doing this, bcmgenet,
>> bcmsysport, tg3 etc.
>
> Thank you. I think I finally got phylib working, more or less.
>
> Unfortunately, it seems I have some kind of race condition. The driver
> has a lot that's wrong with it, and I'm trying to fix it all. One crazy
> the driver does is it create a workqueue to handle a lot of the tasks
> that would normally be handled in the interrupt handler itself.
That sounds like a typicall top half/bottom half split, fair enough.
>
> With phylib support, I know my driver can call phy_mac_interrupt() when
> it gets a link status change interrupt. I then have an .adjust_link
> callback which starts or stops the mac accordingly.
The Ethernet MAC should be started in ndo_open() and stopped in
ndo_close(), in between, there are link state changes, but you are not
supposed to stop or start your Ethernet MAC and its DMA for instance
during link change, if that is a HW requirement, your HW is pretty funky.
>
> My problem is that I'm not really sure what adjust_link is supposed to
> be doing.
Well, it's pretty simple, it is about re-configuring your Ethernet MAC
based on what the PHY link state mandates: duplex, pause, speed changes,
EEE etc is what this callback is supposed to take care of, at the
Ethernet MAC level.
> In addition, it seems that I need to keep the workqueue
> running, otherwise the interface will not function. I bring the
> interface up, and the driver reports success, but pings do not work.
>
> I'm getting really frustrated. The sample code isn't really helping a
> whole lot, because I lack a fundamental understanding of what needs to
> be done. None of the documentation I've read is helpful, and I don't
> know how to debug it.
Seriously, no documentation is helpful? The PHY library seems pretty
well documented to me, but I suppose I have a bias, oh, and patches are
welcome of course.
>
> Can you give me some advice on how to debug this?
Take a look at drivers/net/ethernet/broadcom/genet/bcmgenet.c and see
how it deals with managing link state changes for instance. The code is
pretty straight forward: link interrupt (and other causes) trigger a
workqueue schedule, which then processes link state changes and calls
phy_mac_interrupt(), which in turn makes the PHY library adjust the
interface carrier state.
--
Florian
^ permalink raw reply
* Re: [PATCH net] openvswitch: Fix cached ct with helper.
From: Jarno Rajahalme @ 2016-05-10 23:55 UTC (permalink / raw)
To: Joe Stringer; +Cc: netdev
In-Reply-To: <1462905631-12209-1-git-send-email-joe@ovn.org>
This would result in inconsistent helper assignment if a first CT action assigns a helper and a further CT action tries to assign a different helper; Typically the second helper assignment would be ignored, but if the unconfirmed conntrack entry is lost due to an upcall the second helper assignment would be successful. This is best resolved by allowing helper assignment by a committing CT action only by testing the 'info->commit' flag in addition to the conditions you have there already. It may also be helpful to fail helper assignment without the commit flag in parse_ct().
Jarno
> On May 10, 2016, at 11:40 AM, Joe Stringer <joe@ovn.org> wrote:
>
> When using conntrack helpers from OVS, a common configuration is to
> perform a lookup without specifying a helper, then go through a
> firewalling policy, only to decide to attach a helper afterwards.
>
> In this case, the initial lookup will cause a ct entry to be attached to
> the skb, then the later commit with helper should attach the helper and
> confirm the connection. However, the helper attachment has been missing.
> If the user has enabled automatic helper attachment, then this issue
> will be masked as it will be applied in init_conntrack(). It is also
> masked if the action is executed from ovs_packet_cmd_execute() as that
> will construct a fresh skb.
>
> This patch fixes the issue by making an explicit call to try to assign
> the helper if there is a discrepancy between the action's helper and the
> current skb->nfct.
>
> Fixes: cae3a2627520 ("openvswitch: Allow attaching helpers to ct action")
> Signed-off-by: Joe Stringer <joe@ovn.org>
> ---
> net/openvswitch/conntrack.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
> index b5fea1101faa..89f61a1720eb 100644
> --- a/net/openvswitch/conntrack.c
> +++ b/net/openvswitch/conntrack.c
> @@ -776,6 +776,18 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key,
> return -EINVAL;
> }
>
> + /* Userspace may decide to perform a ct lookup without a helper
> + * specified followed by a (recirculate and) commit with one.
> + * Therefore, for unconfirmed connections we need to attach the
> + * helper here.
> + */
> + if (!nf_ct_is_confirmed(ct) && info->helper && !nfct_help(ct)) {
> + int err = __nf_ct_try_assign_helper(ct, info->ct,
> + GFP_ATOMIC);
> + if (err)
> + return err;
> + }
> +
> /* Call the helper only if:
> * - nf_conntrack_in() was executed above ("!cached") for a
> * confirmed connection, or
> --
> 2.1.4
>
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2016-05-11 0:11 UTC (permalink / raw)
To: David Miller, netdev
Cc: linux-next, linux-kernel, Florian Westphal, Pablo Neira Ayuso
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/netfilter/nf_conntrack_core.c
between commit:
70d72b7e060e ("netfilter: conntrack: init all_locks to avoid debug warning")
from the net tree and commit:
a3efd81205b1 ("netfilter: conntrack: move generation seqcnt out of netns_ct")
56d52d4892d0 ("netfilter: conntrack: use a single hashtable for all namespaces")
0c5366b3a8c7 ("netfilter: conntrack: use single slab cache")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/netfilter/nf_conntrack_core.c
index 895d11dced3c,566c64e3ec50..000000000000
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@@ -66,7 -69,12 +69,12 @@@ EXPORT_SYMBOL_GPL(nf_conntrack_locks)
__cacheline_aligned_in_smp DEFINE_SPINLOCK(nf_conntrack_expect_lock);
EXPORT_SYMBOL_GPL(nf_conntrack_expect_lock);
+ struct hlist_nulls_head *nf_conntrack_hash __read_mostly;
+ EXPORT_SYMBOL_GPL(nf_conntrack_hash);
+
+ static __read_mostly struct kmem_cache *nf_conntrack_cachep;
-static __read_mostly spinlock_t nf_conntrack_locks_all_lock;
+static __read_mostly DEFINE_SPINLOCK(nf_conntrack_locks_all_lock);
+ static __read_mostly seqcount_t nf_conntrack_generation;
static __read_mostly bool nf_conntrack_locks_all;
void nf_conntrack_lock(spinlock_t *lock) __acquires(lock)
^ permalink raw reply
* Re: [PATCH net] openvswitch: Fix cached ct with helper.
From: Joe Stringer @ 2016-05-11 0:31 UTC (permalink / raw)
To: Jarno Rajahalme; +Cc: netdev
In-Reply-To: <67865008-7CDC-469D-8EAA-C097C19F7408@ovn.org>
On 10 May 2016 at 16:55, Jarno Rajahalme <jarno@ovn.org> wrote:
> This would result in inconsistent helper assignment if a first CT action assigns a helper and a further CT action tries to assign a different helper; Typically the second helper assignment would be ignored, but if the unconfirmed conntrack entry is lost due to an upcall the second helper assignment would be successful. This is best resolved by allowing helper assignment by a committing CT action only by testing the 'info->commit' flag in addition to the conditions you have there already. It may also be helpful to fail helper assignment without the commit flag in parse_ct().
Strictly speaking I think that skb_nfct_cached() handles at least some
of those cases but I agree it should be more explicit here. I'll send
a v2. We can follow up separately on net-next to improve the
parsing/make that more user-friendly.
^ permalink raw reply
* Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support
From: Simon Horman @ 2016-05-11 1:50 UTC (permalink / raw)
To: Jiri Benc; +Cc: ovs dev, Linux Kernel Network Developers
In-Reply-To: <20160510140106.367936fb@griffin>
Hi Jiri,
On Tue, May 10, 2016 at 02:01:06PM +0200, Jiri Benc wrote:
> On Mon, 9 May 2016 17:04:22 +0900, Simon Horman wrote:
> > It seems to be caused by the following:
> >
> > 1. __ipgre_rcv() calls skb_pop_mac_header() which
> > sets skb->mac_header to the skb->network_header.
> >
> > 2. __ipgre_rcv() then calls ip_tunnel_rcv() which calls
> > skb_reset_network_header(). This updates skb->network_header to
> > just after the end of the GRE header.
> >
> > This is 28 bytes after the old skb->network_header
> > as there is a 20 byte IPv4 header followed by an
> > 8 byte GRE header.
> >
> > 3. Later, dev_gro_receive() calls skb_reset_mac_len().
> > This calculates skb->mac_len based on skb->network_header and
> > skb->mac_header. I.e. 28 bytes.
>
> Right. Thanks for tracking this down!
>
> > I think this may be possible to address by calling
> > skb_reset_network_header() instead of skb_pop_mac_header()
> > in __ipgre_rcv().
>
> We can't do that. The interface type is ARPHRD_IPGRE and not
> ARPHRD_NONE, so the current behavior makes pretty good sense. See
> e.g. commit 0e3da5bb8da45.
>
> We have two options here:
>
> 1. As for metadata tunnels all the info is in metadata_dst and we
> don't need the IP/GRE header for anything, we can make the ipgre
> interface ARPHRD_NONE in metadata based mode.
>
> 2. We can fix this up in ovs after receiving the packet from
> ARPHRD_IPGRE interface.
>
> I think the first option is the correct one. We already don't assign
> dev->header_ops in metadata mode. I'll prepare a patch.
I agree that 1. seems to be the better approach.
> > Its possible that I've overlooked something but as things stand I think
> > things look like this:
> >
> > * ovs_flow_key_extract() keys off dev->type and skb->protocol.
> > * ovs_flow_key_extract() calls key_extract() which amongst other things
> > sets up the skb->mac_header and skb->mac_len of the skb.
> > * ovs_flow_key_extract() sets skb->protocol to that of the inner packet
> > in the case of TEB
> > * Actions update the above mentioned skb fields as appropriate.
>
> Okay, that actually eases things somewhat.
>
> > So it seems to me that it should be safe to rely on skb->protocol
> > in the receive path. Or more specifically, in netdev_port_receive().
> >
> > If mac_len is also able to be used then I think fine. But it seems to me
> > that it needs to be set up by OvS at least for the ARPHRD_NONE case. This
> > could be done early on, say in netdev_port_receive(). But it seems that
> > would involve duplicating some of what is already occurring in
> > key_extract().
>
> I'd actually prefer doing this earlier, netdev_port_receive looks like
> the right place. Just set mac_len there (or whatever) and let
> key_extract do the rest of the work, not depending on dev->type in
> there.
>
> My point about recirculation was not actually valid, as I missed you're
> doing this in ovs_flow_key_extract and not in key_extract. Still,
> I think the special handling of particular interface types belongs to
> the tx processing on those interfaces, not to the common code.
Sure, if that is your preference I think it should be simple enough to
implement. I agree that netdev_port_receive() looks like a good place for
this.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: [PATCH 1/2] [v4] net: emac: emac gigabit ethernet controller driver
From: Timur Tabi @ 2016-05-11 2:24 UTC (permalink / raw)
To: Florian Fainelli, netdev, linux-kernel, devicetree, linux-arm-msm,
sdharia, Shanker Donthineni, Greg Kroah-Hartman, vikrams, cov,
gavidov, Rob Herring, andrew, bjorn.andersson, Mark Langsdorf,
Jon Masters, Andy Gross, David S. Miller
In-Reply-To: <57326E08.40803@gmail.com>
Florian Fainelli wrote:
> The Ethernet MAC should be started in ndo_open() and stopped in
> ndo_close(), in between, there are link state changes, but you are not
> supposed to stop or start your Ethernet MAC and its DMA for instance
> during link change, if that is a HW requirement, your HW is pretty funky.
I think the problem is that the current driver seems to be too eager to
start/stop the MAC.
Please take a look at emac_work_thread_link_check() at
https://lkml.org/lkml/2016/4/13/670. Every time the PHY link goes up,
it does this:
if (phy->link_up) {
if (netif_carrier_ok(netdev))
goto link_task_done;
pm_runtime_get_sync(netdev->dev.parent);
netif_info(adpt, timer, adpt->netdev, "NIC Link is Up %s\n",
speed);
emac_mac_start(adpt);
netif_carrier_on(netdev);
netif_wake_queue(netdev);
The call to emac_mac_start seems wrong to me here.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
^ permalink raw reply
* Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support
From: Simon Horman @ 2016-05-11 3:06 UTC (permalink / raw)
To: Jiri Benc; +Cc: ovs dev, Linux Kernel Network Developers
In-Reply-To: <20160511015009.GB24436-IxS8c3vjKQDk1uMJSBkQmQ@public.gmane.org>
Hi Jiri,
On Wed, May 11, 2016 at 10:50:09AM +0900, Simon Horman wrote:
[...]
> > > Its possible that I've overlooked something but as things stand I think
> > > things look like this:
> > >
> > > * ovs_flow_key_extract() keys off dev->type and skb->protocol.
> > > * ovs_flow_key_extract() calls key_extract() which amongst other things
> > > sets up the skb->mac_header and skb->mac_len of the skb.
> > > * ovs_flow_key_extract() sets skb->protocol to that of the inner packet
> > > in the case of TEB
> > > * Actions update the above mentioned skb fields as appropriate.
> >
> > Okay, that actually eases things somewhat.
> >
> > > So it seems to me that it should be safe to rely on skb->protocol
> > > in the receive path. Or more specifically, in netdev_port_receive().
> > >
> > > If mac_len is also able to be used then I think fine. But it seems to me
> > > that it needs to be set up by OvS at least for the ARPHRD_NONE case. This
> > > could be done early on, say in netdev_port_receive(). But it seems that
> > > would involve duplicating some of what is already occurring in
> > > key_extract().
> >
> > I'd actually prefer doing this earlier, netdev_port_receive looks like
> > the right place. Just set mac_len there (or whatever) and let
> > key_extract do the rest of the work, not depending on dev->type in
> > there.
> >
> > My point about recirculation was not actually valid, as I missed you're
> > doing this in ovs_flow_key_extract and not in key_extract. Still,
> > I think the special handling of particular interface types belongs to
> > the tx processing on those interfaces, not to the common code.
>
> Sure, if that is your preference I think it should be simple enough to
> implement. I agree that netdev_port_receive() looks like a good place for
> this.
So far I have the following, which seems to work
changes to make dev->type ARPHRD_NONE for compat GRE vports.
Is this close to what you had in mind?
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index d320c2657627..4d2698596033 100644
--- a/net/openvswitch/flow.c
+++ b/net/openvswitch/flow.c
@@ -700,8 +700,6 @@ int ovs_flow_key_update(struct sk_buff *skb, struct sw_flow_key *key)
int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
struct sk_buff *skb, struct sw_flow_key *key)
{
- bool is_layer3 = false;
- bool is_teb = false;
int err;
/* Extract metadata from packet. */
@@ -709,14 +707,6 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
key->tun_proto = ip_tunnel_info_af(tun_info);
memcpy(&key->tun_key, &tun_info->key, sizeof(key->tun_key));
- if (OVS_CB(skb)->input_vport->dev->type != ARPHRD_ETHER) {
- if (skb->protocol == htons(ETH_P_TEB))
- is_teb = true;
- else
- is_layer3 = true;
- }
-
-
if (tun_info->options_len) {
BUILD_BUG_ON((1 << (sizeof(tun_info->options_len) *
8)) - 1
@@ -739,17 +729,17 @@ int ovs_flow_key_extract(const struct ip_tunnel_info *tun_info,
key->phy.skb_mark = skb->mark;
ovs_ct_fill_key(skb, key);
key->ovs_flow_hash = 0;
- key->phy.is_layer3 = is_layer3;
+ key->phy.is_layer3 = (tun_info && skb->mac_len == 0);
key->recirc_id = 0;
err = key_extract(skb, key);
if (err < 0)
return err;
- if (is_teb)
- skb->protocol = key->eth.type;
- else if (is_layer3)
+ if (key->phy.is_layer3)
key->eth.type = skb->protocol;
+ else if (tun_info && skb->protocol == htons(ETH_P_TEB))
+ skb->protocol = key->eth.type;
return err;
}
diff --git a/net/openvswitch/vport-netdev.c b/net/openvswitch/vport-netdev.c
index 7d54414b35eb..ac8178ac2c81 100644
--- a/net/openvswitch/vport-netdev.c
+++ b/net/openvswitch/vport-netdev.c
@@ -60,7 +60,21 @@ static void netdev_port_receive(struct sk_buff *skb)
if (vport->dev->type == ARPHRD_ETHER) {
skb_push(skb, ETH_HLEN);
skb_postpush_rcsum(skb, skb->data, ETH_HLEN);
+ } else if (vport->dev->type == ARPHRD_NONE) {
+ if (skb->protocol == htons(ETH_P_TEB)) {
+ struct ethhdr *eth = eth_hdr(skb);
+
+ if (unlikely(skb->len < ETH_HLEN))
+ goto error;
+
+ skb->mac_len = ETH_HLEN;
+ if (eth->h_proto == htons(ETH_P_8021Q))
+ skb->mac_len += VLAN_HLEN;
+ } else {
+ skb->mac_len = 0;
+ }
}
+
ovs_vport_receive(vport, skb, skb_tunnel_info(skb));
return;
error:
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply related
* Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-R1.0-R2.0
From: Scott Wood @ 2016-05-11 3:26 UTC (permalink / raw)
To: Arnd Bergmann, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, Russell King,
Santosh Shilimkar, Bhupesh Sharma,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kumar Gala,
linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Yang-Leo Li,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
Rob Herring, linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Claudiu Manoil, Yangbo Lu,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Xi
In-Reply-To: <5159406.HOFrj7pRfW@wuerfel>
On Thu, 2016-05-05 at 13:10 +0200, Arnd Bergmann wrote:
> On Thursday 05 May 2016 09:41:32 Yangbo Lu wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> > > Sent: Thursday, May 05, 2016 4:32 PM
> > > To: linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> > > Cc: Yangbo Lu; linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > > linux-clk-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> > > foundation.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Mark Rutland;
> > > ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org; Russell King; Bhupesh Sharma; Joerg Roedel;
> > > Santosh Shilimkar; Yang-Leo Li; Scott Wood; Rob Herring; Claudiu Manoil;
> > > Kumar Gala; Xiaobo Xie; Qiang Zhao
> > > Subject: Re: [v10, 7/7] mmc: sdhci-of-esdhc: fix host version for T4240-
> > > R1.0-R2.0
> > >
> > > On Thursday 05 May 2016 11:12:30 Yangbo Lu wrote:
> > > > IIRC, it is the same IP block as i.MX and Arnd's point is this won't
> > > > even compile on !PPC. It is things like this that prevent sharing the
> > > > driver.
> >
> > The whole point of using the MMIO SVR instead of the PPC SPR is so that
> > it will work on ARM... The guts driver should build on any platform as
> > long as OF is enabled, and if it doesn't find a node to bind to it will
> > return 0 for SVR, and the eSDHC driver will continue (after printing an
> > error that should be removed) without the ability to test for errata
> > based on SVR.
>
> It feels like a bad design to have to come up with a different
> method for each SoC type here when they all do the same thing
> and want to identify some variant of the chip to do device
> specific quirks.
>
> As far as I'm concerned, every driver in drivers/soc that needs to
> export a symbol to be used by a device driver is an indication that
> we don't have the right set of abstractions yet. There are cases
> that are not worth abstracting because the functionality is rather
> obscure and only a couple of drivers for one particular chip
> ever need it.
>
> Finding out the version of the SoC does not look like this case.
I'm open to new ways of abstracting this, but can that please be discussed
after these patches are merged? This patchset is fixing a problem, the
existing abstraction is unappealing and not widely adopted, a new abstraction
is not ready, and we're only touching code for our hardware.
Oh, and the existing abstraction isn't even "existing". I don't see any
examples where soc_device is being used like this -- or even any way for a
driver (the one consuming the information, not the soc "driver") to get a
reference to the soc_device that's been registered short of searching for the
device object by name -- and you're asking for new functionality in
drivers/base/soc.c.
> > > I think the first four patches take care of building for ARM,
> > > but the problem remains if you want to enable COMPILE_TEST as
> > > we need for certain automated checking.
> >
> > What specific problem is there with COMPILE_TEST?
>
> COMPILE_TEST is solvable here and the way it is implemented in this
> case (selecting FSL_GUTS from the driver) indeed looks like it works
> correctly, but it's still awkward that this means building the
> SoC specific ID stuff into the vmlinux binary for any driver that
> uses something like that for a particular SoC.
Please keep in mind that this is a Freescale-specific driver... it's not as if
we're attaching this dependency to common SDHCI code.
>
> > > > Dealing with Si revs is a common problem. We should have a
> > > > common solution. There is soc_device for this purpose.
> > >
> > > Exactly. The last time this came up, I think we agreed to implement a
> > > helper using glob_match() on the soc_device strings. Unfortunately
> > > this hasn't happened then, but I'd still prefer that over yet another
> > > vendor-specific way of dealing with the generic issue.
> >
> > soc_device would require encoding the SVR as a string and then decoding
> > the string, which is more complicated and error prone than having
> > platform-specific code test a platform-specific number.
>
> You already need to encode it as a string to register the soc_device,
No we don't, because we don't already register a soc_device on arm64 or ppc
(and it looks like whatever does get registered on at least some relevant
arm32 chips is not particularly useful).
> and the driver just needs to pass a glob string, so the only part that
> is missing is the generic function that takes the string from the
> driver and passes that to glob_match for the soc_device.
"just"
And what would the glob look like?
I'd rather not write kernel code as if it were a shell/Perl script.
> > And when would it get registered on arm64, which doesn't have
> > platform code?
>
> Whenever the soc driver is loaded, as is the case now. The match
> function can return -EPROBE_DEFER if no SoC device is registered
> yet.
That's too late for some places where we need access to SVR, e.g. clock
drivers (which use CLK_OF_DECLARE and are initialized very early, not as part
of the driver model and thus can't defer). Currently we have an #ifdef
CONFIG_PPC for this in drivers/clk/clk-qoriq.c... Maybe we should have done
that here as well, and saved some grief. :-) At least until an erratum pops
up on an ARM-based chip.
And what happens if we're running on arm32, and thus the arch code already
registered an soc_device with a different (and less useful) encoding?
-Scott
^ permalink raw reply
* Re: [PATCH v9 net-next 4/7] openvswitch: add layer 3 flow/port support
From: Simon Horman @ 2016-05-11 3:28 UTC (permalink / raw)
To: Jiri Benc; +Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160510140618.3ad4a35f@griffin>
Hi Jiri,
On Tue, May 10, 2016 at 02:06:18PM +0200, Jiri Benc wrote:
> On Mon, 9 May 2016 17:18:20 +0900, Simon Horman wrote:
> > On Fri, May 06, 2016 at 11:35:04AM +0200, Jiri Benc wrote:
> > > In addition, we should check whether mac_len > 0 and in such case,
> > > change skb->protocol to ETH_P_TEB first (and store that value in the
> > > pushed eth header).
> > >
> > > Similarly on pop_eth, we need to check skb->protocol and if it is
> > > ETH_P_TEB, call eth_type_trans on the modified frame to set the new
> > > skb->protocol correctly. It's probably not that simple, as we'd need a
> > > version of eth_type_trans that doesn't need a net device.
> >
> > I'm not sure I understand the interaction with ETH_P_TEB here.
> >
> > In my mind skb->protocol == ETH_P_TEB may be used early on in OvS's receive
> > processing to find the inner protocol from the packet and at that point
> > skb->protocol is set to that value. And that for further packet processing
> > the fact the packet was received as TEB is transparent.
>
> Yes but think about the case when you have two Ethernet headers pushed.
>
> We can either disallow it, or do what I described.
>
> Specifically, let's assume we have an IP packet with an Ethernet
> header present. skb->protocol is ETH_P_IP. Now, when there's skb_push,
> the correct operation would be setting skb->protocol to ETH_P_TEB,
> pushing a new Ethernet header and filing ETH_P_TEB to the ethertype
> field in the new header. The packet is not ETH_P_IP anymore, as the L2
> header is followed by another Ethernet header now.
Thanks for the clarification, I had not considered the case of two
ethernet headers when I wrote my previous email.
I think that at this stage I would prefer to prohibit push_eth() acting
on a packet which already has an ethernet header. Indeed that is what
my patch-set already does in its modifications of __ovs_nla_copy_actions().
The reason that I lean towards prohibiting this is that I do not
have an easy way to exercise this case within the current patch-set.
And thus this extra complexity seems well suited to being handled handled
incrementally as further work.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* Re: pull request: batman-adv 20160511
From: David Miller @ 2016-05-11 3:36 UTC (permalink / raw)
To: a
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <1462908605-27412-1-git-send-email-a-2CpIooy/SPIKlTDg6p0iyA@public.gmane.org>
From: Antonio Quartulli <a@unstable.cc>
Date: Wed, 11 May 2016 03:29:48 +0800
> here you have a pull request intended for net-next.
> There are 17 patches in this batch, but most of them are cleanups
> and minor code re-arrangement.
>
> The more detailed description follows in the git tag.
>
> Please pull or let me know of any problem.
Pulled, thanks Antonio.
^ permalink raw reply
* Re: [PATCH v3 net-next] tcp: replace cnt & rtt with struct in pkts_acked()
From: David Miller @ 2016-05-11 3:47 UTC (permalink / raw)
To: brakmo; +Cc: netdev, kernel-team, ncardwell, eric.dumazet, ycheng
In-Reply-To: <1462911069-1834733-1-git-send-email-brakmo@fb.com>
From: Lawrence Brakmo <brakmo@fb.com>
Date: Tue, 10 May 2016 13:11:09 -0700
> Replace 2 arguments (cnt and rtt) in the congestion control modules'
> pkts_acked() function with a struct. This will allow adding more
> information without having to modify existing congestion control
> modules (tcp_nv in particular needs bytes in flight when packet
> was sent).
>
> As proposed by Neal Cardwell in his comments to the tcp_nv patch.
>
> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
> Acked-by: Yuchung Cheng <ycheng@google.com>
Applied, thanks.
^ permalink raw reply
* RE: [PATCH] net: phylib: fix interrupts re-enablement in phy_start
From: Shaohui Xie @ 2016-05-11 2:15 UTC (permalink / raw)
To: Florian Fainelli, netdev@vger.kernel.org, davem@davemloft.net; +Cc: Andrew Lunn
In-Reply-To: <57322792.4060008@gmail.com>
> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Wednesday, May 11, 2016 2:25 AM
> To: shh.xie@gmail.com; netdev@vger.kernel.org; davem@davemloft.net
> Cc: Shaohui Xie <shaohui.xie@nxp.com>; Andrew Lunn <andrew@lunn.ch>
> Subject: Re: [PATCH] net: phylib: fix interrupts re-enablement in phy_start
>
> On 05/10/2016 02:42 AM, shh.xie@gmail.com wrote:
> > From: Shaohui Xie <Shaohui.Xie@nxp.com>
> >
> > If phy was suspended and is starting, current driver always enable
> > phy's interrupts, if phy works in polling, phy can raise unexpected
> > interrupt which will not be handled, the interrupt will block system
> > enter suspend again. So interrupts should only be re-enabled if phy
> > works in interrupt.
>
> Your explanation makes sense. The commit message could you use some
> improvements like s/phy/PHY/ and "works in interrupt mode", but that is not a
> huge thing.
[S.H] Thank you for the comment.
I should make the commit message more precise and will make improvement in
future patch submission.
Regards,
Shaohui
^ permalink raw reply
* Re: [PATCH v3 net-next] tcp: replace cnt & rtt with struct in pkts_acked()
From: David Miller @ 2016-05-11 3:49 UTC (permalink / raw)
To: brakmo; +Cc: netdev, kernel-team, ncardwell, eric.dumazet, ycheng
In-Reply-To: <20160510.234758.925276118117758879.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Tue, 10 May 2016 23:47:58 -0400 (EDT)
> From: Lawrence Brakmo <brakmo@fb.com>
> Date: Tue, 10 May 2016 13:11:09 -0700
>
>> Replace 2 arguments (cnt and rtt) in the congestion control modules'
>> pkts_acked() function with a struct. This will allow adding more
>> information without having to modify existing congestion control
>> modules (tcp_nv in particular needs bytes in flight when packet
>> was sent).
>>
>> As proposed by Neal Cardwell in his comments to the tcp_nv patch.
>>
>> Signed-off-by: Lawrence Brakmo <brakmo@fb.com>
>> Acked-by: Yuchung Cheng <ycheng@google.com>
>
> Applied, thanks.
Actually I had to revert, this breaks the build:
net/ipv4/tcp_illinois.c: In function ‘tcp_illinois_acked’:
net/ipv4/tcp_illinois.c:97:18: error: assignment of member ‘rtt_us’ in read-only object
sample->rtt_us = RTT_MAX;
You can't mark this argument const.
^ permalink raw reply
* Re: [net PATCH v2 0/6] net sched: Fix broken late binding of actions
From: David Miller @ 2016-05-11 3:50 UTC (permalink / raw)
To: jhs; +Cc: netdev, xiyou.wangcong, jiri, alexander.duyck
In-Reply-To: <1462913371-9699-1-git-send-email-jhs@emojatatu.com>
From: Jamal Hadi Salim <jhs@mojatatu.com>
Date: Tue, 10 May 2016 16:49:25 -0400
> Some actions were broken in allowing for late binding of actions.
> Late binding workflow is as follows:
> a) create an action and provide all necessary parameters for it
> Optionally provide an index or let the kernel give you one.
> Example:
> sudo tc actions add action police rate 1kbit burst 90k drop index 1
>
> b) later on bind to the pre-created action from a filter definition
> by merely specifying the index.
> Example:
> sudo tc filter add dev lo parent ffff: protocol ip prio 8 \
> u32 match ip src 127.0.0.8/32 flowid 1:8 action police index 1
Series applied, thanks Jamal.
^ permalink raw reply
* Re: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero
From: Mark D Rustad @ 2016-05-11 3:59 UTC (permalink / raw)
To: Jarod Wilson
Cc: Mark Rustad, Denys Vlasenko, netdev@vger.kernel.org,
intel-wired-lan@lists.osuosl.org, LKML
In-Reply-To: <20160510210042.GA19319@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 2091 bytes --]
Jarod Wilson <jarod@redhat.com> wrote:
> On Fri, May 06, 2016 at 11:43:17PM +0000, Rustad, Mark D wrote:
>> Denys Vlasenko <dvlasenk@redhat.com> wrote:
>>
>>> Users report that under VMWare, er32(TIMINCA) returns zero.
>>> This causes division by zero at init time as follows:
>>>
>>> ==> incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
>>> for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
>>> /* latch SYSTIMH on read of SYSTIML */
>>> systim_next = (cycle_t)er32(SYSTIML);
>>> systim_next |= (cycle_t)er32(SYSTIMH) << 32;
>>>
>>> time_delta = systim_next - systim;
>>> temp = time_delta;
>>> ====> rem = do_div(temp, incvalue);
>>>
>>> This change makes kernel survive this, and users report that
>>> NIC does work after this change.
>>>
>>> Since on real hardware incvalue is never zero, this should not affect
>>> real hardware use case.
> ...
>> I seem to recall that this was rejected before because it really is
>> VMWare's
>> bug and, if they fix it, any existing VMs that use this will just work.
>> Changing the driver will only fix it for vms that install a new driver. I
>> don't object to doing it, it just seems like not the most effective
>> place to
>> address the issue.
>
> You could also have people who never update VMWare, for whom a kernel
> work-around would be better. I think it'd be best to address it both at
> the driver level and the emulated hardware level, to improve things for
> the most possible users. Those who update neither hypervisor or
> kernel/driver, well, they reap what they sow.
That is a sound argument for doing both. I would expect that there are more
frozen VM images than host environments, but I can certainly imagine that
some choose to freeze their host. Of course if everything is frozen there
is no point at all. :-)
I am on an extended vacation, and don't work on e1000e anyway, so I will
quit my kibitzing here.
--
Mark Rustad, MRustad@gmail.com
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
^ permalink raw reply
* Re: [PATCH] cfg80211/nl80211: add wifi tx power mode switching support
From: Wei-Ning Huang @ 2016-05-11 5:03 UTC (permalink / raw)
To: Dan Williams
Cc: Linux-Wireless, LKML, Johannes Berg, Sameer Nanda, Todd Broch,
davem, netdev
In-Reply-To: <CABicQ-VWrRG+0rEEF0GxUTPixSbkA18Gq4b5hbgph60SoEQ+2Q@mail.gmail.com>
On Fri, May 6, 2016 at 4:19 PM, Wei-Ning Huang <wnhuang@google.com> wrote:
> On Fri, May 6, 2016 at 12:07 AM, Dan Williams <dcbw@redhat.com> wrote:
>>
>> On Thu, 2016-05-05 at 14:44 +0800, Wei-Ning Huang wrote:
>> > Recent new hardware has the ability to switch between tablet mode and
>> > clamshell mode. To optimize WiFi performance, we want to be able to
>> > use
>> > different power table between modes. This patch adds a new netlink
>> > message type and cfg80211_ops function to allow userspace to trigger
>> > a
>> > power mode switch for a given wireless interface.
>> >
>> > Signed-off-by: Wei-Ning Huang <wnhuang@chromium.org>
>> > ---
>> > include/net/cfg80211.h | 11 +++++++++++
>> > include/uapi/linux/nl80211.h | 21 +++++++++++++++++++++
>> > net/wireless/nl80211.c | 16 ++++++++++++++++
>> > net/wireless/rdev-ops.h | 22 ++++++++++++++++++++++
>> > net/wireless/trace.h | 20 ++++++++++++++++++++
>> > 5 files changed, 90 insertions(+)
>> >
>> > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
>> > index 9e1b24c..aa77fa0 100644
>> > --- a/include/net/cfg80211.h
>> > +++ b/include/net/cfg80211.h
>> > @@ -2370,6 +2370,12 @@ struct cfg80211_qos_map {
>> > * @get_tx_power: store the current TX power into the dbm variable;
>> > * return 0 if successful
>> > *
>> > + * @set_tx_power_mode: set the transmit power mode. Some device have
>> > the ability
>> > + * to transform between different mode such as clamshell and
>> > tablet mode.
>> > + * set_tx_power_mode allows setting of different TX power
>> > mode at runtime.
>> > + * @get_tx_power_mode: store the current TX power mode into the mode
>> > variable;
>> > + * return 0 if successful
>> > + *
>> > * @set_wds_peer: set the WDS peer for a WDS interface
>> > *
>> > * @rfkill_poll: polls the hw rfkill line, use cfg80211 reporting
>> > @@ -2631,6 +2637,11 @@ struct cfg80211_ops {
>> > int (*get_tx_power)(struct wiphy *wiphy, struct
>> > wireless_dev *wdev,
>> > int *dbm);
>> >
>> > + int (*set_tx_power_mode)(struct wiphy *wiphy,
>> > + enum nl80211_tx_power_mode
>> > mode);
>> > + int (*get_tx_power_mode)(struct wiphy *wiphy,
>> > + enum nl80211_tx_power_mode
>> > *mode);
>> > +
>> > int (*set_wds_peer)(struct wiphy *wiphy, struct
>> > net_device *dev,
>> > const u8 *addr);
>> >
>> > diff --git a/include/uapi/linux/nl80211.h
>> > b/include/uapi/linux/nl80211.h
>> > index 5a30a75..9b1888a 100644
>> > --- a/include/uapi/linux/nl80211.h
>> > +++ b/include/uapi/linux/nl80211.h
>> > @@ -1796,6 +1796,9 @@ enum nl80211_commands {
>> > * connecting to a PCP, and in %NL80211_CMD_START_AP to start
>> > * a PCP instead of AP. Relevant for DMG networks only.
>> > *
>> > + * @NL80211_ATTR_WIPHY_TX_POWER_MODE: Transmit power mode. See
>> > + * &enum nl80211_tx_power_mode for possible values.
>> > + *
>> > * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>> > * @NL80211_ATTR_MAX: highest attribute number currently defined
>> > * @__NL80211_ATTR_AFTER_LAST: internal use
>> > @@ -2172,6 +2175,8 @@ enum nl80211_attrs {
>> >
>> > NL80211_ATTR_PBSS,
>> >
>> > + NL80211_ATTR_WIPHY_TX_POWER_MODE,
>> > +
>> > /* add attributes here, update the policy in nl80211.c */
>> >
>> > __NL80211_ATTR_AFTER_LAST,
>> > @@ -3703,6 +3708,22 @@ enum nl80211_tx_power_setting {
>> > };
>> >
>> > /**
>> > + * enum nl80211_tx_power_mode - TX power mode setting
>> > + * @NL80211_TX_POWER_LOW: general low TX power mode
>> > + * @NL80211_TX_POWER_MEDIUM: general medium TX power mode
>> > + * @NL80211_TX_POWER_HIGH: general high TX power mode
>> > + * @NL80211_TX_POWER_CLAMSHELL: clamshell mode TX power mode
>> > + * @NL80211_TX_POWER_TABLET: tablet mode TX power mode
>> > + */
>> > +enum nl80211_tx_power_mode {
>> > + NL80211_TX_POWER_LOW,
>> > + NL80211_TX_POWER_MEDIUM,
>> > + NL80211_TX_POWER_HIGH,
>> > + NL80211_TX_POWER_CLAMSHELL,
>> > + NL80211_TX_POWER_TABLET,
>>
>
>> "clamshell" and "tablet" probably mean many different things to many
>> different people with respect to whether or not they should do anything
>> with power saving or wifi. I feel like a more generic interface is
>> needed here.
> We could probably drop those two CLAMSHELL and TABLET constant or
> describing what they mean
> in more detail?
>
>>
>> Could this be already done by:
>> @NL80211_ATTR_WIPHY_TX_POWER_SETTING = NL80211_TX_POWER_LIMITED
>> @NL80211_ATTR_WIPHY_TX_POWER_LEVEL = <limit for clamshell/tablet mode>
>>
>> and then the device would be able to change its TX power as it saw fit
>> up to that limit set by your application which figures out whether it's
>> in clamshell or tablet mode?
>
> We usually want different power settings in different band/channel.
> For example, we can have three different power settings
> in 2.4Ghz, channels 36-64 & channels 100+ on 5Ghz. In this case, we
> can not simply set a fixed number to the power level.
> A power table is required to map channel/band to actual power limit.
> For most of the driver, changing power table requires loading
> a new set of calibration data from either devicetree or ACPI. Due to
> this, a new message type is required to allow the driver to
> switch between tables.
>
> Wei-Ning
Hi Dan,
Does this make sense to you? If so I'll send another patch to clarify
the term clamshell / tablet mode.
Thanks for reviewing.
Wei-Ning
>>
>>
>> Dan
>>
>> > +};
>> > +
>> > +/**
>> > * enum nl80211_packet_pattern_attr - packet pattern attribute
>> > * @__NL80211_PKTPAT_INVALID: invalid number for nested attribute
>> > * @NL80211_PKTPAT_PATTERN: the pattern, values where the mask has
>> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> > index 056a730..61b474d 100644
>> > --- a/net/wireless/nl80211.c
>> > +++ b/net/wireless/nl80211.c
>> > @@ -402,6 +402,7 @@ static const struct nla_policy
>> > nl80211_policy[NUM_NL80211_ATTR] = {
>> > [NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 },
>> > [NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG },
>> > [NL80211_ATTR_PBSS] = { .type = NLA_FLAG },
>> > + [NL80211_ATTR_WIPHY_TX_POWER_MODE] = { .type = NLA_U32 },
>> > };
>> >
>> > /* policy for the key attributes */
>> > @@ -2218,6 +2219,21 @@ static int nl80211_set_wiphy(struct sk_buff
>> > *skb, struct genl_info *info)
>> > return result;
>> > }
>> >
>> > + if (info->attrs[NL80211_ATTR_WIPHY_TX_POWER_MODE]) {
>> > + enum nl80211_tx_power_mode mode;
>> > + int idx = 0;
>> > +
>> > + if (!rdev->ops->set_tx_power_mode)
>> > + return -EOPNOTSUPP;
>> > +
>> > + idx = NL80211_ATTR_WIPHY_TX_POWER_MODE;
>> > + mode = nla_get_u32(info->attrs[idx]);
>> > +
>> > + result = rdev_set_tx_power_mode(rdev, mode);
>> > + if (result)
>> > + return result;
>> > + }
>> > +
>> > if (info->attrs[NL80211_ATTR_WIPHY_ANTENNA_TX] &&
>> > info->attrs[NL80211_ATTR_WIPHY_ANTENNA_RX]) {
>> > u32 tx_ant, rx_ant;
>> > diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
>> > index 8ae0c04..c3a1035 100644
>> > --- a/net/wireless/rdev-ops.h
>> > +++ b/net/wireless/rdev-ops.h
>> > @@ -552,6 +552,28 @@ static inline int rdev_get_tx_power(struct
>> > cfg80211_registered_device *rdev,
>> > return ret;
>> > }
>> >
>> > +static inline int
>> > +rdev_set_tx_power_mode(struct cfg80211_registered_device *rdev,
>> > + enum nl80211_tx_power_mode mode)
>> > +{
>> > + int ret;
>> > + trace_rdev_set_tx_power_mode(&rdev->wiphy, mode);
>> > + ret = rdev->ops->set_tx_power_mode(&rdev->wiphy, mode);
>> > + trace_rdev_return_int(&rdev->wiphy, ret);
>> > + return ret;
>> > +}
>> > +
>> > +static inline int
>> > +rdev_get_tx_power_mode(struct cfg80211_registered_device *rdev,
>> > + enum nl80211_tx_power_mode *mode)
>> > +{
>> > + int ret;
>> > + trace_rdev_get_tx_power_mode(&rdev->wiphy);
>> > + ret = rdev->ops->get_tx_power_mode(&rdev->wiphy, mode);
>> > + trace_rdev_return_int_int(&rdev->wiphy, ret, *mode);
>> > + return ret;
>> > +}
>> > +
>> > static inline int rdev_set_wds_peer(struct
>> > cfg80211_registered_device *rdev,
>> > struct net_device *dev, const u8
>> > *addr)
>> > {
>> > diff --git a/net/wireless/trace.h b/net/wireless/trace.h
>> > index 09b242b..132c8c2 100644
>> > --- a/net/wireless/trace.h
>> > +++ b/net/wireless/trace.h
>> > @@ -1420,6 +1420,26 @@ TRACE_EVENT(rdev_set_tx_power,
>> > WIPHY_PR_ARG, WDEV_PR_ARG,__entry->type, __entry-
>> > >mbm)
>> > );
>> >
>> > +DEFINE_EVENT(wiphy_only_evt, rdev_get_tx_power_mode,
>> > + TP_PROTO(struct wiphy *wiphy),
>> > + TP_ARGS(wiphy)
>> > +);
>> > +
>> > +TRACE_EVENT(rdev_set_tx_power_mode,
>> > + TP_PROTO(struct wiphy *wiphy, enum nl80211_tx_power_mode
>> > mode),
>> > + TP_ARGS(wiphy, mode),
>> > + TP_STRUCT__entry(
>> > + WIPHY_ENTRY
>> > + __field(enum nl80211_tx_power_mode, mode)
>> > + ),
>> > + TP_fast_assign(
>> > + WIPHY_ASSIGN;
>> > + __entry->mode = mode;
>> > + ),
>> > + TP_printk(WIPHY_PR_FMT ", mode: %d",
>> > + WIPHY_PR_ARG, __entry->mode)
>> > +);
>> > +
>> > TRACE_EVENT(rdev_return_int_int,
>> > TP_PROTO(struct wiphy *wiphy, int func_ret, int func_fill),
>> > TP_ARGS(wiphy, func_ret, func_fill),
>
>
>
>
> --
> Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
> wnhuang@google.com | Cell: +886 910-380678
--
Wei-Ning Huang, 黃偉寧 | Software Engineer, Google Inc., Taiwan |
wnhuang@google.com | Cell: +886 910-380678
^ permalink raw reply
* Re: [PATCH nf-next,v2] gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)
From: Cong Wang @ 2016-05-11 5:24 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Linux Kernel Network Developers, David Miller, laforge, aschultz,
openbsc
In-Reply-To: <1462748148-17764-1-git-send-email-pablo@netfilter.org>
On Sun, May 8, 2016 at 3:55 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> +static int gtp_genl_new_pdp(struct sk_buff *skb, struct genl_info *info)
> +{
...
> +
> + net = gtp_genl_get_net(sock_net(skb->sk), info->attrs);
> + if (IS_ERR(net))
> + return PTR_ERR(net);
> +
> + /* Check if there's an existing gtpX device to configure */
> + dev = gtp_find_dev(net, nla_get_u32(info->attrs[GTPA_LINK]));
> + if (dev == NULL)
> + return -ENODEV;
> +
> + return ipv4_pdp_add(dev, info);
Seems you are leaking struct net at least in the error path.
^ permalink raw reply
* [PATCH -next 0/4] net: w5100: collection of small changes
From: Akinobu Mita @ 2016-05-11 6:30 UTC (permalink / raw)
To: netdev; +Cc: Akinobu Mita
This patch series for w5100 driver includes a cleanup with no functional
change, two fixes, and adding a functionality. Each change is relatively
small.
Akinobu Mita (4):
net: w5100: remove unused is_w5200()
net: w5100: fix MAC filtering for W5500
net: w5100: increase TX timeout period
net: w5100-spi: add support to specify MAC address by device tree
drivers/net/ethernet/wiznet/w5100-spi.c | 4 +++-
drivers/net/ethernet/wiznet/w5100.c | 29 +++++++++++++++++------------
drivers/net/ethernet/wiznet/w5100.h | 3 ++-
3 files changed, 22 insertions(+), 14 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH -next 1/4] net: w5100: remove unused is_w5200()
From: Akinobu Mita @ 2016-05-11 6:30 UTC (permalink / raw)
To: netdev; +Cc: Akinobu Mita, Mike Sinkovsky, David S . Miller
In-Reply-To: <1462948227-21276-1-git-send-email-akinobu.mita@gmail.com>
The is_w5200() function is not used anymore by the commit which adds
the W5500 support.
Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Mike Sinkovsky <msink@permonline.ru>
Cc: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/wiznet/w5100.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/net/ethernet/wiznet/w5100.c b/drivers/net/ethernet/wiznet/w5100.c
index 8ed0c77..56ceed9 100644
--- a/drivers/net/ethernet/wiznet/w5100.c
+++ b/drivers/net/ethernet/wiznet/w5100.c
@@ -173,11 +173,6 @@ struct w5100_priv {
struct work_struct restart_work;
};
-static inline bool is_w5200(struct w5100_priv *priv)
-{
- return priv->ops->chip_id == W5200;
-}
-
/************************************************************************
*
* Lowlevel I/O functions
--
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