* Re: [PATCH 16/51] DMA-API: ppc: vio.c: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Cedric Le Goater @ 2013-11-15 16:16 UTC (permalink / raw)
To: Russell King
Cc: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
linux-wireless, netdev, Solarflare linux maintainers,
uclinux-dist-devel, Paul Mackerras, Benjamin Herrenschmidt
In-Reply-To: <E1VMly8-0007gy-Ru@rmk-PC.arm.linux.org.uk>
Hi,
On 09/19/2013 11:41 PM, Russell King wrote:
> Replace the following sequence:
>
> dma_set_mask(dev, mask);
> dma_set_coherent_mask(dev, mask);
>
> with a call to the new helper dma_set_mask_and_coherent().
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> arch/powerpc/kernel/vio.c | 3 +--
> 1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
> index 78a3506..96b6c97 100644
> --- a/arch/powerpc/kernel/vio.c
> +++ b/arch/powerpc/kernel/vio.c
> @@ -1413,8 +1413,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
>
> /* needed to ensure proper operation of coherent allocations
> * later, in case driver doesn't set it explicitly */
> - dma_set_mask(&viodev->dev, DMA_BIT_MASK(64));
> - dma_set_coherent_mask(&viodev->dev, DMA_BIT_MASK(64));
> + dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
> }
>
> /* register with generic device framework */
>
The new helper routine dma_set_mask_and_coherent() breaks the
initialization of the pseries vio devices which do not have an
initial dev->dma_mask. I think we need to use dma_coerce_mask_and_coherent()
instead.
Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
arch/powerpc/kernel/vio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vio.c b/arch/powerpc/kernel/vio.c
index e7d0c88f..76a6482 100644
--- a/arch/powerpc/kernel/vio.c
+++ b/arch/powerpc/kernel/vio.c
@@ -1419,7 +1419,7 @@ struct vio_dev *vio_register_device_node(struct device_node *of_node)
/* needed to ensure proper operation of coherent allocations
* later, in case driver doesn't set it explicitly */
- dma_set_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
+ dma_coerce_mask_and_coherent(&viodev->dev, DMA_BIT_MASK(64));
}
/* register with generic device framework */
--
1.7.10.4
^ permalink raw reply related
* Re: [RFC PATCH net-next] net: reorder struct netns_ct for better cache-line usage
From: Eric Dumazet @ 2013-11-15 16:17 UTC (permalink / raw)
To: Jesper Dangaard Brouer; +Cc: Pablo Neira Ayuso, netdev, netfilter-devel
In-Reply-To: <20131115145459.17641.35021.stgit@dragon>
On Fri, 2013-11-15 at 15:57 +0100, Jesper Dangaard Brouer wrote:
> Reorder struct netns_ct so that atomic_t "count" changes don't
> slowdown users of read mostly fields.
>
> This is based on Eric Dumazet's proposed patch:
> "netfilter: conntrack: remove the central spinlock"
> http://thread.gmane.org/gmane.linux.network/268758/focus=47306
>
> The tricky part of cache-aligning this structure, that it is getting
> inlined in struct net (include/net/net_namespace.h), thus changes to
> other netns_xxx structures affects our alignment.
>
> Eric's original patch contained an ambiguity on 32-bit regarding
> alignment in struct net. This patch also takes 32-bit into account,
> and in case of changed (struct net) alignment sysctl_xxx entries have
> been ordered according to how often they are accessed.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> Reviewed-by: Jiri Benc <jbenc@redhat.com>
> ---
> Yes, I know it's not the time to submit this patch, but
> I just want to let Eric know that I'm planning on extracting
> part of his proposed patch, which have been stalled for months.
> I'll resubmit once net-next/nf-next opens for changes again.
I have no plan working on conntrack in the near future.
I cannot really cope with a 3 months response time to a patch.
Feel free to grab and continue the work, thanks !
^ permalink raw reply
* Re: [PATCH v2.50 1/6] ofp-actions: Consider L4 actions after mpls_push as inconsistent
From: Jarno Rajahalme @ 2013-11-15 16:55 UTC (permalink / raw)
To: Simon Horman
Cc: dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ravi K
In-Reply-To: <1384506623-18503-2-git-send-email-horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
Looks good to me,
Acked-by: Jarno Rajahalme <jrajahalme-l0M0P4e3n4LQT0dZR+AlfA@public.gmane.org>
> On Nov 15, 2013, at 1:10 AM, Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org> wrote:
>
> After an mpls_push action the resulting packet is MPLS and
> the MPLS payload is opaque. Thus nothing can be assumed
> about the packets network protocol and it is inconsistent
> to apply L4 actions.
>
> With regards to actions that affect the packet at other layers
> of the protocol stack:
>
> * L3: The consistency of L3 actions should already be handled correctly
> by virtue of the dl_type of the flow being temporarily altered
> during consistency checking by both push_mpls and pop_mpls actions.
>
> * MPLS: The consistency checking of MPLS actions appear to already be
> handled correctly.
>
> * VLAN: At this time Open vSwitch on mpls_push an MPLS LSE is always
> added after any VLAN tags that follow the ethernet header.
> That is the tag ordering defined prior to OpenFlow1.3. As such
> VLAN actions should sill be equally valid before and after mpls_push
> and mpls_pop actions.
>
> * L2 actions are equally valid before and after mpls_push and mpls_pop actions.
>
> Signed-off-by: Simon Horman <horms-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
>
> ---
> v2.50
> * Correct typo in comment. nw_protocol -> nw_proto
>
> v2.49
> * First post
> ---
> lib/ofp-actions.c | 9 ++++++++-
> tests/ofp-actions.at | 12 ++++++++++++
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index abc9505..03ad1a4 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -1869,7 +1869,8 @@ ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports)
> }
> }
>
> -/* May modify flow->dl_type and flow->vlan_tci, caller must restore them.
> +/* May modify flow->dl_type, flow->nw_proto and flow->vlan_tci,
> + * caller must restore them.
> *
> * Modifies some actions, filling in fields that could not be properly set
> * without context. */
> @@ -2056,6 +2057,10 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
>
> case OFPACT_PUSH_MPLS:
> flow->dl_type = ofpact_get_PUSH_MPLS(a)->ethertype;
> + /* The packet is now MPLS and the MPLS payload is opaque.
> + * Thus nothing can be assumed about the network protocol.
> + * Temporarily mark that we have no nw_proto. */
> + flow->nw_proto = 0;
> return 0;
>
> case OFPACT_POP_MPLS:
> @@ -2127,6 +2132,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> struct ofpact *a;
> ovs_be16 dl_type = flow->dl_type;
> ovs_be16 vlan_tci = flow->vlan_tci;
> + uint8_t nw_proto = flow->nw_proto;
> enum ofperr error = 0;
>
> OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
> @@ -2139,6 +2145,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> /* Restore fields that may have been modified. */
> flow->dl_type = dl_type;
> flow->vlan_tci = vlan_tci;
> + flow->nw_proto = nw_proto;
> return error;
> }
>
> diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> index cdca8ca..08ebccf 100644
> --- a/tests/ofp-actions.at
> +++ b/tests/ofp-actions.at
> @@ -477,3 +477,15 @@ AT_CHECK(
> [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-ofp11-instructions < input.txt],
> [0], [expout], [experr])
> AT_CLEANUP
> +
> +AT_SETUP([ofp-actions - inconsistent MPLS actions])
> +OVS_VSWITCHD_START
> +dnl OK: Use fin_timeout action on TCP flow
> +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)'])
> +dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS
> +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'],
> + [1], [], [dnl
> +ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT)
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
> --
> 1.8.4
>
> _______________________________________________
> dev mailing list
> dev-yBygre7rU0TnMu66kgdUjQ@public.gmane.org
> http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH 1/2] pkt_sched: fq: warn users using defrate
From: Eric Dumazet @ 2013-11-15 16:57 UTC (permalink / raw)
To: David Miller
Cc: netdev, Maciej Żenczykowski, Willem de Bruijn, Yuchung Cheng,
Neal Cardwell
From: Eric Dumazet <edumazet@google.com>
Commit 7eec4174ff29 ("pkt_sched: fq: fix non TCP flows pacing")
obsoleted TCA_FQ_FLOW_DEFAULT_RATE without notice for the users.
Suggested by David Miller
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
include/uapi/linux/pkt_sched.h | 4 +---
net/sched/sch_fq.c | 10 ++++------
2 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 307f293477e8..885001b62c83 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -763,9 +763,7 @@ enum {
TCA_FQ_RATE_ENABLE, /* enable/disable rate limiting */
- TCA_FQ_FLOW_DEFAULT_RATE,/* for sockets with unspecified sk_rate,
- * use the following rate
- */
+ TCA_FQ_FLOW_DEFAULT_RATE,/* obsolete, do not use */
TCA_FQ_FLOW_MAX_RATE, /* per flow max rate */
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index d4fa38e4af80..26906ab51c52 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -88,7 +88,6 @@ struct fq_sched_data {
struct fq_flow internal; /* for non classified or high prio packets */
u32 quantum;
u32 initial_quantum;
- u32 flow_default_rate;/* rate per flow : bytes per second */
u32 flow_max_rate; /* optional max rate per flow */
u32 flow_plimit; /* max packets per flow */
struct rb_root *fq_root;
@@ -650,7 +649,8 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt)
q->initial_quantum = nla_get_u32(tb[TCA_FQ_INITIAL_QUANTUM]);
if (tb[TCA_FQ_FLOW_DEFAULT_RATE])
- q->flow_default_rate = nla_get_u32(tb[TCA_FQ_FLOW_DEFAULT_RATE]);
+ pr_warn_ratelimited("sch_fq: defrate %u ignored.\n",
+ nla_get_u32(tb[TCA_FQ_FLOW_DEFAULT_RATE]));
if (tb[TCA_FQ_FLOW_MAX_RATE])
q->flow_max_rate = nla_get_u32(tb[TCA_FQ_FLOW_MAX_RATE]);
@@ -699,7 +699,6 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt)
q->flow_plimit = 100;
q->quantum = 2 * psched_mtu(qdisc_dev(sch));
q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
- q->flow_default_rate = 0;
q->flow_max_rate = ~0U;
q->rate_enable = 1;
q->new_flows.first = NULL;
@@ -726,9 +725,8 @@ static int fq_dump(struct Qdisc *sch, struct sk_buff *skb)
if (opts == NULL)
goto nla_put_failure;
- /* TCA_FQ_FLOW_DEFAULT_RATE is not used anymore,
- * do not bother giving its value
- */
+ /* TCA_FQ_FLOW_DEFAULT_RATE is not used anymore */
+
if (nla_put_u32(skb, TCA_FQ_PLIMIT, sch->limit) ||
nla_put_u32(skb, TCA_FQ_FLOW_PLIMIT, q->flow_plimit) ||
nla_put_u32(skb, TCA_FQ_QUANTUM, q->quantum) ||
^ permalink raw reply related
* [PATCH 2/2] pkt_sched: fq: fix pacing for small frames
From: Eric Dumazet @ 2013-11-15 16:58 UTC (permalink / raw)
To: David Miller
Cc: netdev, Maciej Żenczykowski, Willem de Bruijn, Yuchung Cheng,
Neal Cardwell
From: Eric Dumazet <edumazet@google.com>
For performance reasons, sch_fq tried hard to not setup timers for every
sent packet, using a quantum based heuristic : A delay is setup only if
the flow exhausted its credit.
Problem is that application limited flows can refill their credit
for every queued packet, and they can evade pacing.
This problem can also be triggered when TCP flows use small MSS values,
as TSO auto sizing builds packets that are smaller than the default fq
quantum (3028 bytes)
This patch adds a 40 ms delay to guard flow credit refill.
Fixes: afe4fd062416 ("pkt_sched: fq: Fair Queue packet scheduler")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Maciej Żenczykowski <maze@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: Yuchung Cheng <ycheng@google.com>
Cc: Neal Cardwell <ncardwell@google.com>
---
include/uapi/linux/pkt_sched.h | 3 +++
net/sched/sch_fq.c | 22 ++++++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 885001b62c83..a806687ad98f 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -768,6 +768,9 @@ enum {
TCA_FQ_FLOW_MAX_RATE, /* per flow max rate */
TCA_FQ_BUCKETS_LOG, /* log2(number of buckets) */
+
+ TCA_FQ_FLOW_REFILL_DELAY, /* flow credit refill delay in usec */
+
__TCA_FQ_MAX
};
diff --git a/net/sched/sch_fq.c b/net/sched/sch_fq.c
index 26906ab51c52..95d843961907 100644
--- a/net/sched/sch_fq.c
+++ b/net/sched/sch_fq.c
@@ -88,6 +88,7 @@ struct fq_sched_data {
struct fq_flow internal; /* for non classified or high prio packets */
u32 quantum;
u32 initial_quantum;
+ u32 flow_refill_delay;
u32 flow_max_rate; /* optional max rate per flow */
u32 flow_plimit; /* max packets per flow */
struct rb_root *fq_root;
@@ -114,6 +115,7 @@ static struct fq_flow detached, throttled;
static void fq_flow_set_detached(struct fq_flow *f)
{
f->next = &detached;
+ f->age = jiffies;
}
static bool fq_flow_is_detached(const struct fq_flow *f)
@@ -366,17 +368,20 @@ static int fq_enqueue(struct sk_buff *skb, struct Qdisc *sch)
}
f->qlen++;
- flow_queue_add(f, skb);
if (skb_is_retransmit(skb))
q->stat_tcp_retrans++;
sch->qstats.backlog += qdisc_pkt_len(skb);
if (fq_flow_is_detached(f)) {
fq_flow_add_tail(&q->new_flows, f);
- if (q->quantum > f->credit)
- f->credit = q->quantum;
+ if (time_after(jiffies, f->age + q->flow_refill_delay))
+ f->credit = max_t(u32, f->credit, q->quantum);
q->inactive_flows--;
qdisc_unthrottled(sch);
}
+
+ /* Note: this overwrites f->age */
+ flow_queue_add(f, skb);
+
if (unlikely(f == &q->internal)) {
q->stat_internal_packets++;
qdisc_unthrottled(sch);
@@ -454,7 +459,6 @@ begin:
fq_flow_add_tail(&q->old_flows, f);
} else {
fq_flow_set_detached(f);
- f->age = jiffies;
q->inactive_flows++;
}
goto begin;
@@ -608,6 +612,7 @@ static const struct nla_policy fq_policy[TCA_FQ_MAX + 1] = {
[TCA_FQ_FLOW_DEFAULT_RATE] = { .type = NLA_U32 },
[TCA_FQ_FLOW_MAX_RATE] = { .type = NLA_U32 },
[TCA_FQ_BUCKETS_LOG] = { .type = NLA_U32 },
+ [TCA_FQ_FLOW_REFILL_DELAY] = { .type = NLA_U32 },
};
static int fq_change(struct Qdisc *sch, struct nlattr *opt)
@@ -664,6 +669,12 @@ static int fq_change(struct Qdisc *sch, struct nlattr *opt)
err = -EINVAL;
}
+ if (tb[TCA_FQ_FLOW_REFILL_DELAY]) {
+ u32 usecs_delay = nla_get_u32(tb[TCA_FQ_FLOW_REFILL_DELAY]) ;
+
+ q->flow_refill_delay = usecs_to_jiffies(usecs_delay);
+ }
+
if (!err)
err = fq_resize(q, fq_log);
@@ -699,6 +710,7 @@ static int fq_init(struct Qdisc *sch, struct nlattr *opt)
q->flow_plimit = 100;
q->quantum = 2 * psched_mtu(qdisc_dev(sch));
q->initial_quantum = 10 * psched_mtu(qdisc_dev(sch));
+ q->flow_refill_delay = msecs_to_jiffies(40);
q->flow_max_rate = ~0U;
q->rate_enable = 1;
q->new_flows.first = NULL;
@@ -733,6 +745,8 @@ static int fq_dump(struct Qdisc *sch, struct sk_buff *skb)
nla_put_u32(skb, TCA_FQ_INITIAL_QUANTUM, q->initial_quantum) ||
nla_put_u32(skb, TCA_FQ_RATE_ENABLE, q->rate_enable) ||
nla_put_u32(skb, TCA_FQ_FLOW_MAX_RATE, q->flow_max_rate) ||
+ nla_put_u32(skb, TCA_FQ_FLOW_REFILL_DELAY,
+ jiffies_to_usecs(q->flow_refill_delay)) ||
nla_put_u32(skb, TCA_FQ_BUCKETS_LOG, q->fq_trees_log))
goto nla_put_failure;
^ permalink raw reply related
* Re: [ovs-dev] [PATCH v2.50 1/6] ofp-actions: Consider L4 actions after mpls_push as inconsistent
From: Ben Pfaff @ 2013-11-15 17:45 UTC (permalink / raw)
To: Jarno Rajahalme
Cc: Simon Horman, dev@openvswitch.org, netdev@vger.kernel.org,
Jesse Gross, Ravi K
In-Reply-To: <6A09EE11-CDFA-4E49-AB7F-34746ED4765D@nicira.com>
Thanks Simon, thanks Jarno, applied.
On Fri, Nov 15, 2013 at 08:55:42AM -0800, Jarno Rajahalme wrote:
> Looks good to me,
>
> Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
>
> > On Nov 15, 2013, at 1:10 AM, Simon Horman <horms@verge.net.au> wrote:
> >
> > After an mpls_push action the resulting packet is MPLS and
> > the MPLS payload is opaque. Thus nothing can be assumed
> > about the packets network protocol and it is inconsistent
> > to apply L4 actions.
> >
> > With regards to actions that affect the packet at other layers
> > of the protocol stack:
> >
> > * L3: The consistency of L3 actions should already be handled correctly
> > by virtue of the dl_type of the flow being temporarily altered
> > during consistency checking by both push_mpls and pop_mpls actions.
> >
> > * MPLS: The consistency checking of MPLS actions appear to already be
> > handled correctly.
> >
> > * VLAN: At this time Open vSwitch on mpls_push an MPLS LSE is always
> > added after any VLAN tags that follow the ethernet header.
> > That is the tag ordering defined prior to OpenFlow1.3. As such
> > VLAN actions should sill be equally valid before and after mpls_push
> > and mpls_pop actions.
> >
> > * L2 actions are equally valid before and after mpls_push and mpls_pop actions.
> >
> > Signed-off-by: Simon Horman <horms@verge.net.au>
> >
> > ---
> > v2.50
> > * Correct typo in comment. nw_protocol -> nw_proto
> >
> > v2.49
> > * First post
> > ---
> > lib/ofp-actions.c | 9 ++++++++-
> > tests/ofp-actions.at | 12 ++++++++++++
> > 2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> > index abc9505..03ad1a4 100644
> > --- a/lib/ofp-actions.c
> > +++ b/lib/ofp-actions.c
> > @@ -1869,7 +1869,8 @@ ofpact_check_output_port(ofp_port_t port, ofp_port_t max_ports)
> > }
> > }
> >
> > -/* May modify flow->dl_type and flow->vlan_tci, caller must restore them.
> > +/* May modify flow->dl_type, flow->nw_proto and flow->vlan_tci,
> > + * caller must restore them.
> > *
> > * Modifies some actions, filling in fields that could not be properly set
> > * without context. */
> > @@ -2056,6 +2057,10 @@ ofpact_check__(struct ofpact *a, struct flow *flow,
> >
> > case OFPACT_PUSH_MPLS:
> > flow->dl_type = ofpact_get_PUSH_MPLS(a)->ethertype;
> > + /* The packet is now MPLS and the MPLS payload is opaque.
> > + * Thus nothing can be assumed about the network protocol.
> > + * Temporarily mark that we have no nw_proto. */
> > + flow->nw_proto = 0;
> > return 0;
> >
> > case OFPACT_POP_MPLS:
> > @@ -2127,6 +2132,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> > struct ofpact *a;
> > ovs_be16 dl_type = flow->dl_type;
> > ovs_be16 vlan_tci = flow->vlan_tci;
> > + uint8_t nw_proto = flow->nw_proto;
> > enum ofperr error = 0;
> >
> > OFPACT_FOR_EACH (a, ofpacts, ofpacts_len) {
> > @@ -2139,6 +2145,7 @@ ofpacts_check(struct ofpact ofpacts[], size_t ofpacts_len,
> > /* Restore fields that may have been modified. */
> > flow->dl_type = dl_type;
> > flow->vlan_tci = vlan_tci;
> > + flow->nw_proto = nw_proto;
> > return error;
> > }
> >
> > diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> > index cdca8ca..08ebccf 100644
> > --- a/tests/ofp-actions.at
> > +++ b/tests/ofp-actions.at
> > @@ -477,3 +477,15 @@ AT_CHECK(
> > [ovs-ofctl '-vPATTERN:console:%c|%p|%m' parse-ofp11-instructions < input.txt],
> > [0], [expout], [experr])
> > AT_CLEANUP
> > +
> > +AT_SETUP([ofp-actions - inconsistent MPLS actions])
> > +OVS_VSWITCHD_START
> > +dnl OK: Use fin_timeout action on TCP flow
> > +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=fin_timeout(idle_timeout=1)'])
> > +dnl Bad: Use fin_timeout action on TCP flow that has been converted to MPLS
> > +AT_CHECK([ovs-ofctl -O OpenFlow11 -vwarn add-flow br0 'tcp actions=push_mpls:0x8847,fin_timeout(idle_timeout=1)'],
> > + [1], [], [dnl
> > +ovs-ofctl: actions are invalid with specified match (OFPBAC_MATCH_INCONSISTENT)
> > +])
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > --
> > 1.8.4
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev
^ permalink raw reply
* [PATCH 1/1] fixup! ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses
From: Thomas Haller @ 2013-11-15 17:48 UTC (permalink / raw)
To: jiri, netdev
Cc: davem, kuznet, jmorris, yoshfuji, kaber, thaller, stephen, hannes,
vyasevich, dcbw
In-Reply-To: <1384181359-23199-3-git-send-email-jiri@resnulli.us>
Handle IFA_F_MANAGETEMPADDR flag when modifying existing addresses too.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
Hi,
I'd like to suggest this change, I think it makes it easier for the user space
application to set this flag of (potentially) existing addresses.
Note, that if the flag gets cleared if the ifa had it set before
and the temporary addresses will exprire immediatly. This makes sense
for me.
Or if we don't want this, then the command should fail (EEXIST?) when trying
to set MANAGETEMPADDR for an existing address that doesn't have the flag already.
Thomas
net/ipv6/addrconf.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 9e217b8..9e726c8 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -3613,20 +3613,21 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh)
return inet6_addr_del(net, ifm->ifa_index, pfx, ifm->ifa_prefixlen);
}
static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
u32 prefered_lft, u32 valid_lft)
{
u32 flags;
clock_t expires;
unsigned long timeout;
+ int mngtmp;
if (!valid_lft || (prefered_lft > valid_lft))
return -EINVAL;
timeout = addrconf_timeout_fixup(valid_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
expires = jiffies_to_clock_t(timeout * HZ);
valid_lft = timeout;
flags = RTF_EXPIRES;
} else {
@@ -3636,35 +3637,48 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
}
timeout = addrconf_timeout_fixup(prefered_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
if (timeout == 0)
ifa_flags |= IFA_F_DEPRECATED;
prefered_lft = timeout;
}
spin_lock_bh(&ifp->lock);
- ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS)) | ifa_flags;
+ if (ifa_flags & IFA_F_MANAGETEMPADDR &&
+ (ifp->prefix_len != 64 || ifp->flags & IFA_F_TEMPORARY)) {
+ spin_unlock_bh(&ifp->lock);
+ return -EEXIST;
+ }
+ mngtmp = ifp->flags & IFA_F_MANAGETEMPADDR;
+ ifp->flags = (ifp->flags & ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD | IFA_F_HOMEADDRESS |
+ IFA_F_MANAGETEMPADDR)) | ifa_flags;
ifp->tstamp = jiffies;
ifp->valid_lft = valid_lft;
ifp->prefered_lft = prefered_lft;
spin_unlock_bh(&ifp->lock);
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
expires, flags);
- if (ifp->flags & IFA_F_MANAGETEMPADDR)
- manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
+ if (mngtmp && !(ifa_flags & IFA_F_MANAGETEMPADDR)) {
+ /* expire the life time of the temporary addresses */
+ manage_tempaddrs(ifp->idev, ifp, 0, 0,
false, jiffies);
+ } else if (ifa_flags & IFA_F_MANAGETEMPADDR) {
+ /* if we did not yet mngtmp, create temporary addresses */
+ manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
+ !mngtmp, jiffies);
+ }
addrconf_verify(0);
return 0;
}
static int
inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
{
struct net *net = sock_net(skb->sk);
@@ -3710,23 +3724,20 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
if (ifa == NULL) {
/*
* It would be best to check for !NLM_F_CREATE here but
* userspace alreay relies on not having to provide this.
*/
return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
ifm->ifa_prefixlen, ifa_flags,
preferred_lft, valid_lft);
}
- /* We ignore IFA_F_MANAGETEMPADDR for modify. */
- ifa_flags &= ~IFA_F_MANAGETEMPADDR;
-
if (nlh->nlmsg_flags & NLM_F_EXCL ||
!(nlh->nlmsg_flags & NLM_F_REPLACE))
err = -EEXIST;
else
err = inet6_addr_modify(ifa, ifa_flags, preferred_lft, valid_lft);
in6_ifa_put(ifa);
return err;
}
--
1.8.4.1.559.gdb9bdfb
^ permalink raw reply related
* Re: [PATCH net] bonding: set primary_reselect in LB and AB mode
From: Jay Vosburgh @ 2013-11-15 18:02 UTC (permalink / raw)
To: Ding Tianhong
Cc: Andy Gospodarek, David S. Miller, Nikolay Aleksandrov,
Veaceslav Falico, Netdev
In-Reply-To: <5285F130.5060705@huawei.com>
Ding Tianhong <dingtianhong@huawei.com> wrote:
>The primary_reselect only reselection for the primary slave,
>but the primary slave only support for ALB, TLB and AB mode,
>so we sould set the primary_reselect for these mode.
>
>to fix this: Add a check for ALB, TLB and AB mode in
>bonding_store_primary_reselect, avoid to select active slave
>again in other modes.
I don't believe that setting primary_reselect in a
!USES_PRIMARY() mode has any negative effects. It doesn't do anything,
but also doesn't break anything. Is there a case that setting
primary_reselect causes misbehavior in a !USES_PRIMARY() mode?
Presuming that primary_reselect doesn't break things, this just
adds an ordering limitation when configuring bonding (mode must be set
prior to primary_reselect). I don't believe this change adds any value,
and may break existing configuration scripts.
-J
>Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
>---
> drivers/net/bonding/bond_sysfs.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
>diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>index 47749c9..cac2291 100644
>--- a/drivers/net/bonding/bond_sysfs.c
>+++ b/drivers/net/bonding/bond_sysfs.c
>@@ -1141,6 +1141,13 @@ static ssize_t bonding_store_primary_reselect(struct device *d,
> if (!rtnl_trylock())
> return restart_syscall();
>
>+ if (!USES_PRIMARY(bond->params.mode)) {
>+ pr_info("%s: Unable to set primary reselect; %s is in mode %d\n",
>+ bond->dev->name, bond->dev->name, bond->params.mode);
>+ ret = -EINVAL;
>+ goto out;
>+ }
>+
> new_value = bond_parse_parm(buf, pri_reselect_tbl);
> if (new_value < 0) {
> pr_err("%s: Ignoring invalid primary_reselect value %.*s.\n",
>--
>1.7.12
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [patch net-next RFC 2/2] ipv6 addrconf: introduce IFA_F_MANAGETEMPADDR to tell kernel to manage temporary addresses
From: Thomas Haller @ 2013-11-15 18:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, kuznet, jmorris, yoshfuji, kaber, stephen, hannes,
vyasevich, dcbw
In-Reply-To: <1384181359-23199-3-git-send-email-jiri@resnulli.us>
[-- Attachment #1: Type: text/plain, Size: 895 bytes --]
On Mon, 2013-11-11 at 15:49 +0100, Jiri Pirko wrote:
> Creating an address with this flag set will result in kernel taking care
> of temporary addresses in the same way as if the address was created by
> kernel itself (after RA receive). This allows userspace applications
> implementing the autoconfiguration (NetworkManager for example) to
> implement ipv6 addresses privacy.
Hi,
I think, addrconf_prefix_rcv should not do anything if user created ones
already exist.
"Already exist" could mean, a user created address with:
- the same IP address and MANAGETEMPADDR flag
- the same IP address
- the same /64 prefix and MANAGETEMPADDR flag
- the same /64 prefix
(??)
Of course, the user should disable accept_ra, if he wants to manage
the temporary addresses himself. But it would still be more correct
to check that prefixes from RA do not interfere.
Thomas
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] random: seed random_int_secret at least poorly at core_initcall time
From: Kees Cook @ 2013-11-15 18:33 UTC (permalink / raw)
To: Theodore Ts'o
Cc: Daniel Borkmann, David S. Miller,
shemminger-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, Florian Weimer,
netdev-u79uwXL29TY76Z2rM5mHXA, Eric Dumazet,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131114025448.GB31602-AKGzg7BKzIDYtjvyW6yDsg@public.gmane.org>
On Wed, Nov 13, 2013 at 6:54 PM, Theodore Ts'o <tytso-3s7WtUTddSA@public.gmane.org> wrote:
> On Tue, Nov 12, 2013 at 02:46:03PM +0100, Hannes Frederic Sowa wrote:
>> > It is needed by fork to set up the stack canary. And this actually gets
>> > called before the secret is initialized.
>>
>> Maybe we could use this for the time being and use the seeding method
>> of kaslr as soon as it hits the tree?
>
> Hmm, from what I can tell even early_initcall() is going to be early
> enough. The stack canary is set up by boot_init_stack_canary(), which
> is run very, very early in start_kerne() --- way before
> early_initcalls, or even before interrupts are enabled. So adding
> random_int_secret_init_early() as a core_initcall is still too late.
>
> I wonder if we need to do something in common with what Kees has been
> considering for the kaslr code, since it's a similar issue --- we need
> random number way earlier than we can really afford to initialize
> /dev/random.
Right now I've only been looking at x86. As things currently stand,
we'll make use of RDRAND and RDTSC if they're available, and if
neither are then go to the i8254 timer. Ingo wanted even more
unpredictability, in the face of total failure from these more dynamic
sources, so x86 also "seeds" itself with the build string and the
boot_params. These last two are hardly high entropy, but they should
at least make 2 different systems not have _identical_ entropy at the
start. It's far from cryptographically secure, but it's something, I
hope.
> P.S. Unless I'm missing something (and I hope I am), it would appear
> that the stack canary is going to easily predictable by an attacker on
> non-x86 platforms that don't have RDRAND. Has someone tested whether
> or not the stack canary isn't constant across ARM or pre-Sandy Bridge
> x86 systems?
When the static stack canary was mentioned during the ARM summit, I
dug around a little bit and saw that at very early boot, yes, it was
always the same, but after boot finished, it was different from boot
to boot. I didn't get far enough to figure out what was changing it
later on.
-Kees
--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] random: seed random_int_secret at least poorly at core_initcall time
From: Kees Cook @ 2013-11-15 18:42 UTC (permalink / raw)
To: Theodore Ts'o, Daniel Borkmann, David S. Miller, shemminger,
Florian Weimer, netdev, Eric Dumazet, linux-wireless, Kees Cook
In-Reply-To: <20131114041829.GA26901@order.stressinduktion.org>
On Wed, Nov 13, 2013 at 8:18 PM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> On Wed, Nov 13, 2013 at 09:54:48PM -0500, Theodore Ts'o wrote:
>> On Tue, Nov 12, 2013 at 02:46:03PM +0100, Hannes Frederic Sowa wrote:
>> > > It is needed by fork to set up the stack canary. And this actually gets
>> > > called before the secret is initialized.
>> >
>> > Maybe we could use this for the time being and use the seeding method
>> > of kaslr as soon as it hits the tree?
>>
>> Hmm, from what I can tell even early_initcall() is going to be early
>> enough. The stack canary is set up by boot_init_stack_canary(), which
>> is run very, very early in start_kerne() --- way before
>> early_initcalls, or even before interrupts are enabled. So adding
>> random_int_secret_init_early() as a core_initcall is still too late.
>
> Actually I tried to protect the tsk->stack_canary = get_random_int()
> in fork.c. It sets up the per-task canary.
I haven't looked closely yet at how the stack canary gets plumbed, but
what do things outside of process context end up using?
>> I wonder if we need to do something in common with what Kees has been
>> considering for the kaslr code, since it's a similar issue --- we need
>> random number way earlier than we can really afford to initialize
>> /dev/random.
>
> Definiteley. I would also propose hashing the boot arguments, often
> enough there is a filesystem UUID in there, or even hash the multiboot
> information we are given from grub. Maybe compile-time entropy, at least
> a bit.
Yeah, other suggestions were things ACPI tables, memory layouts, etc.
Basically anything that might be different from system to system. Even
just perturbing by the build strings (which has compiler version,
build date, build host, etc etc) gets us a tiny amount of entropy
(which is still better than 0).
>> P.S. Unless I'm missing something (and I hope I am), it would appear
>> that the stack canary is going to easily predictable by an attacker on
>> non-x86 platforms that don't have RDRAND. Has someone tested whether
>> or not the stack canary isn't constant across ARM or pre-Sandy Bridge
>> x86 systems?
>
> In case of protection for interrupt stacks and early cmwq threads,
> it looks pretty bad from a first look at the source (at least for the
> first initialized CPU).
>
> I'll try to do some tests tomorrow.
>
> Greetings,
>
> Hannes
>
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH] random: seed random_int_secret at least poorly at core_initcall time
From: Dave Jones @ 2013-11-15 18:45 UTC (permalink / raw)
To: Kees Cook
Cc: Theodore Ts'o, Daniel Borkmann, David S. Miller,
shemminger-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, Florian Weimer,
netdev-u79uwXL29TY76Z2rM5mHXA, Eric Dumazet,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAGXu5j+ySEdQBXKkspYC=svfekBja2Z_2tcWSAOEbvyiMLf=aA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 15, 2013 at 10:33:04AM -0800, Kees Cook wrote:
> Ingo wanted even more
> unpredictability, in the face of total failure from these more dynamic
> sources, so x86 also "seeds" itself with the build string and the
> boot_params. These last two are hardly high entropy, but they should
> at least make 2 different systems not have _identical_ entropy at the
> start. It's far from cryptographically secure, but it's something, I
> hope.
Those are both likely to be the same on some configurations.
On x86, we could maybe hash the dmi tables ? Vendor stupidity aside,
things like serial numbers in those tables _should_ be different.
Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] random: seed random_int_secret at least poorly at core_initcall time
From: Kees Cook @ 2013-11-15 19:07 UTC (permalink / raw)
To: Dave Jones
Cc: Theodore Ts'o, Daniel Borkmann, David S. Miller,
Florian Weimer, netdev, Eric Dumazet, linux-wireless
In-Reply-To: <20131115184510.GA911@redhat.com>
On Fri, Nov 15, 2013 at 10:45 AM, Dave Jones <davej@redhat.com> wrote:
> On Fri, Nov 15, 2013 at 10:33:04AM -0800, Kees Cook wrote:
>
> > Ingo wanted even more
> > unpredictability, in the face of total failure from these more dynamic
> > sources, so x86 also "seeds" itself with the build string and the
> > boot_params. These last two are hardly high entropy, but they should
> > at least make 2 different systems not have _identical_ entropy at the
> > start. It's far from cryptographically secure, but it's something, I
> > hope.
>
> Those are both likely to be the same on some configurations.
> On x86, we could maybe hash the dmi tables ? Vendor stupidity aside,
> things like serial numbers in those tables _should_ be different.
Yeah, DMI tables were suggested as well. (Hopefully people will start
using -uuid with KVM!) How hard would that be to hook up to the
pre-random-init code?
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Chang @ 2013-11-15 19:59 UTC (permalink / raw)
To: Neil Horman, Vlad Yasevich
Cc: davem, linux-sctp, netdev, linux-kernel, dreibh, ernstgr
In-Reply-To: <20131115145609.GC29877@hmsreliant.think-freely.org>
I've tried to catch you guys up~
Here's a quick question:
Where are the default [transport->pf_retrans] and
[transport->pathmaxrtx] set? I could figure out that they could be set
through setsockopt(SCTP_PEER_ADDR_THLDS, ...) (but it seems like the
SCTP library has not supported such option yet, maybe that's due to my
library is out of date). So by default both of the two threshold are zero.
Here's my question: Does it go conflict with "the recommended value for
Path.Max.Retrans in the standard is 5"?
Thanks!
On 11/15/2013 03:56 PM, Neil Horman wrote:
> On Fri, Nov 15, 2013 at 09:00:58AM -0500, Vlad Yasevich wrote:
>> On 11/15/2013 07:30 AM, Neil Horman wrote:
>>> On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
>>>> On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
>>>>> Expected Behavior:
>>>>> When hearing an ack from a tranport/path, set its state to normal/on if it's
>>>>> in abnormal(__partial_failure__ or inactive) state.
>>>>>
>>>>> state machine of tranport->state
>>>>> Whenever a T3_RTX timer expires, then transport->error_count++.
>>>>> When (association->pf_retrans < transport->error_count < tranport->pathmaxrtx)
>>>>> transport->state = SCTP_PF //partial failure
>>>>>
>>>>> When a heartbeat-ack comes or conventional ack acknowledged its availability,
>>>>> transport->state = SCTP_ON
>>>>>
>>>>> Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
>>>>> Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from tsvwg")
>>>> I don't think this is right. The spec states:
>>>> 8. ACKs for retransmissions do not transition a PF destination back
>>>> to Active state, since a sender cannot disambiguate whether the
>>>> ack was for the original transmission or the retransmission(s).
>>>>
>>>>
>>>> Now, the proper way to this would would be modify
>>>> sctp_assoc_control_transport() to transition the transport state to
>>>> ACTIVE if it was PF transport that was chosen to send data.
>>>>
>>>> -vlad
>>>>
>>> I agree, this patch doesn't agree with the spec, the only time we transition
>> >from PF to ACTIVE should be on receipt of ack of new data.
>>
>> You mean HB ACK, right? The 02 spec that see on the ietf site doesn't
>> mention anything about transition on SACKs. Also, there is no way to
>> tell right now if the ack is for new or retransmitted data. We could
>> mark chunks that are retransmitted though.
>>
> Yes, sorry I wasn't clear, I was speaknig about HB Acks.
>
>>> I'm not even sure if
>>> we should allow PF transports to be selected to send new data. Currently a
>>> potentially failed transport will get ignored when specified, and the stack will
>>> use the active path in its place. Only if all transports are PF will a PF
>>> transport be chosen.
>> Not even that :(. If all transports are PF, we are going to camp on
>> the primary path instead of choosing a PF transport with the lowest
>> error count.
>>
> Yes, we don't do the smallest error count check, though I have to wonder if
> thats really worthwhile. If all your transports are PF, you're a step away from
> a connection reset anyway.
> Neil
>
>> -vlad
>>
>>> Neil
>>>
>>>>> ---
>>>>> net/sctp/outqueue.c | 1 +
>>>>> 1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>>>> index 94df758..2557fa5 100644
>>>>> --- a/net/sctp/outqueue.c
>>>>> +++ b/net/sctp/outqueue.c
>>>>> @@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
>>>>> * active if it is not so marked.
>>>>> */
>>>>> if ((transport->state == SCTP_INACTIVE ||
>>>>> + transport->state == SCTP_PF ||
>>>>> transport->state == SCTP_UNCONFIRMED) &&
>>>>> sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
>>>>> sctp_assoc_control_transport(
>>>>>
>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
^ permalink raw reply
* [PATCH v2] net: Do not include padding in TCP GRO checksum
From: Alexander Duyck @ 2013-11-15 20:03 UTC (permalink / raw)
To: davem; +Cc: netdev, edumazet, herbert
In some recent tests I found the TCP checksum was being treated as valid
for certain frames with padding on them. On closer inspection I found the
issue was that GRO was using the skb->len instead of the length recorded in
the IP/IPv6 header to determine the number of bytes to checksum. As such
padded frames that actually had invalid checksums generated by adding the
padding to the checksum were being incorrectly tagged as valid.
This change corrects that by using the tot_len from IPv4 headers and the
payload_len from IPv6 headers to compute the total number of bytes to be
included in the checksum.
To address the fact that skb->csum is invalid when a padded frame is
received I have updated the code to fall though to the CHECKSUM_NONE path
for CHECKSUM_COMPLETE frames that contain padding.
Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
---
v2: Update byte ordering of tot_len and payload_len so it is in host order.
Updated CHECKSUM_COMPLETE path so it falls back through CHECKSUM_NONE for
padded frames since this is how it is handled in ip_rcv.
I have tested and verified the CHECKSUM_NONE path works, but I don't have
any adapters that generate CHECKSUM_COMPLETE to test with.
net/ipv4/tcp_offload.c | 26 +++++++++++++++++---------
net/ipv6/tcpv6_offload.c | 27 +++++++++++++++++----------
2 files changed, 34 insertions(+), 19 deletions(-)
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index a2b68a1..17b5d14 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -273,26 +273,34 @@ static int tcp_v4_gso_send_check(struct sk_buff *skb)
static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *skb)
{
const struct iphdr *iph = skb_gro_network_header(skb);
+ int length = ntohs(iph->tot_len);
__wsum wsum;
__sum16 sum;
+ /* adjust for any offsets */
+ length += skb_network_offset(skb) - skb_gro_offset(skb);
+
switch (skb->ip_summed) {
case CHECKSUM_COMPLETE:
- if (!tcp_v4_check(skb_gro_len(skb), iph->saddr, iph->daddr,
- skb->csum)) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- break;
- }
+ if (length == skb_gro_len(skb)) {
+ if (!tcp_v4_check(length, iph->saddr, iph->daddr,
+ skb->csum)) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ break;
+ }
flush:
- NAPI_GRO_CB(skb)->flush = 1;
- return NULL;
+ NAPI_GRO_CB(skb)->flush = 1;
+ return NULL;
+ }
+ /* skb->csum is invalid if frame is padded */
+ skb->ip_summed = CHECKSUM_NONE;
case CHECKSUM_NONE:
wsum = csum_tcpudp_nofold(iph->saddr, iph->daddr,
- skb_gro_len(skb), IPPROTO_TCP, 0);
+ length, IPPROTO_TCP, 0);
sum = csum_fold(skb_checksum(skb,
skb_gro_offset(skb),
- skb_gro_len(skb),
+ length,
wsum));
if (sum)
goto flush;
diff --git a/net/ipv6/tcpv6_offload.c b/net/ipv6/tcpv6_offload.c
index c1097c7..abfa938 100644
--- a/net/ipv6/tcpv6_offload.c
+++ b/net/ipv6/tcpv6_offload.c
@@ -36,27 +36,34 @@ static struct sk_buff **tcp6_gro_receive(struct sk_buff **head,
struct sk_buff *skb)
{
const struct ipv6hdr *iph = skb_gro_network_header(skb);
+ int length = ntohs(iph->payload_len);
__wsum wsum;
__sum16 sum;
+ /* adjust for any offset due to extension headers */
+ length += skb_transport_offset(skb) - skb_gro_offset(skb);
+
switch (skb->ip_summed) {
case CHECKSUM_COMPLETE:
- if (!tcp_v6_check(skb_gro_len(skb), &iph->saddr, &iph->daddr,
- skb->csum)) {
- skb->ip_summed = CHECKSUM_UNNECESSARY;
- break;
- }
+ if (length == skb_gro_len(skb)) {
+ if (!tcp_v6_check(length, &iph->saddr, &iph->daddr,
+ skb->csum)) {
+ skb->ip_summed = CHECKSUM_UNNECESSARY;
+ break;
+ }
flush:
- NAPI_GRO_CB(skb)->flush = 1;
- return NULL;
+ NAPI_GRO_CB(skb)->flush = 1;
+ return NULL;
+ }
+ /* skb->csum is invalid if frame is padded */
+ skb->ip_summed = CHECKSUM_NONE;
case CHECKSUM_NONE:
wsum = ~csum_unfold(csum_ipv6_magic(&iph->saddr, &iph->daddr,
- skb_gro_len(skb),
- IPPROTO_TCP, 0));
+ length, IPPROTO_TCP, 0));
sum = csum_fold(skb_checksum(skb,
skb_gro_offset(skb),
- skb_gro_len(skb),
+ length,
wsum));
if (sum)
goto flush;
^ permalink raw reply related
* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Neil Horman @ 2013-11-15 20:25 UTC (permalink / raw)
To: Chang
Cc: Vlad Yasevich, davem, linux-sctp, netdev, linux-kernel, dreibh,
ernstgr
In-Reply-To: <52867D2E.2090907@gmail.com>
On Fri, Nov 15, 2013 at 08:59:42PM +0100, Chang wrote:
> I've tried to catch you guys up~
>
> Here's a quick question:
>
> Where are the default [transport->pf_retrans] and
> [transport->pathmaxrtx] set? I could figure out that they could be
> set through setsockopt(SCTP_PEER_ADDR_THLDS, ...) (but it seems like
> the SCTP library has not supported such option yet, maybe that's due
Yeah, it looks like Daniel hasn't implemented option 31 yet. You could
certainly issue that socket option on your own thought, without the help of the
library.
> to my library is out of date). So by default both of the two
> threshold are zero.
>
Thats not tre, pathmaxrtx should be initalized to 5, which is what the per-net
sysctl get initalized to in sctp_net_init.
> Here's my question: Does it go conflict with "the recommended value
> for Path.Max.Retrans in the standard is 5"?
>
pathmaxrtx is initalized to 5 in sctp_init_sock, so we're in compliance with the
RFC hre.
> Thanks!
>
> On 11/15/2013 03:56 PM, Neil Horman wrote:
> >On Fri, Nov 15, 2013 at 09:00:58AM -0500, Vlad Yasevich wrote:
> >>On 11/15/2013 07:30 AM, Neil Horman wrote:
> >>>On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
> >>>>On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
> >>>>>Expected Behavior:
> >>>>>When hearing an ack from a tranport/path, set its state to normal/on if it's
> >>>>>in abnormal(__partial_failure__ or inactive) state.
> >>>>>
> >>>>>state machine of tranport->state
> >>>>>Whenever a T3_RTX timer expires, then transport->error_count++.
> >>>>>When (association->pf_retrans < transport->error_count < tranport->pathmaxrtx)
> >>>>> transport->state = SCTP_PF //partial failure
> >>>>>
> >>>>>When a heartbeat-ack comes or conventional ack acknowledged its availability,
> >>>>> transport->state = SCTP_ON
> >>>>>
> >>>>>Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
> >>>>>Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from tsvwg")
> >>>>I don't think this is right. The spec states:
> >>>> 8. ACKs for retransmissions do not transition a PF destination back
> >>>> to Active state, since a sender cannot disambiguate whether the
> >>>> ack was for the original transmission or the retransmission(s).
> >>>>
> >>>>
> >>>>Now, the proper way to this would would be modify
> >>>>sctp_assoc_control_transport() to transition the transport state to
> >>>>ACTIVE if it was PF transport that was chosen to send data.
> >>>>
> >>>>-vlad
> >>>>
> >>>I agree, this patch doesn't agree with the spec, the only time we transition
> >>>from PF to ACTIVE should be on receipt of ack of new data.
> >>
> >>You mean HB ACK, right? The 02 spec that see on the ietf site doesn't
> >>mention anything about transition on SACKs. Also, there is no way to
> >>tell right now if the ack is for new or retransmitted data. We could
> >>mark chunks that are retransmitted though.
> >>
> >Yes, sorry I wasn't clear, I was speaknig about HB Acks.
> >
> >>>I'm not even sure if
> >>>we should allow PF transports to be selected to send new data. Currently a
> >>>potentially failed transport will get ignored when specified, and the stack will
> >>>use the active path in its place. Only if all transports are PF will a PF
> >>>transport be chosen.
> >>Not even that :(. If all transports are PF, we are going to camp on
> >>the primary path instead of choosing a PF transport with the lowest
> >>error count.
> >>
> >Yes, we don't do the smallest error count check, though I have to wonder if
> >thats really worthwhile. If all your transports are PF, you're a step away from
> >a connection reset anyway.
> >Neil
> >
> >>-vlad
> >>
> >>>Neil
> >>>
> >>>>>---
> >>>>> net/sctp/outqueue.c | 1 +
> >>>>> 1 file changed, 1 insertion(+)
> >>>>>
> >>>>>diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> >>>>>index 94df758..2557fa5 100644
> >>>>>--- a/net/sctp/outqueue.c
> >>>>>+++ b/net/sctp/outqueue.c
> >>>>>@@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct sctp_outq *q,
> >>>>> * active if it is not so marked.
> >>>>> */
> >>>>> if ((transport->state == SCTP_INACTIVE ||
> >>>>>+ transport->state == SCTP_PF ||
> >>>>> transport->state == SCTP_UNCONFIRMED) &&
> >>>>> sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
> >>>>> sctp_assoc_control_transport(
> >>>>>
> >>>>
> >>--
> >>To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
>
>
^ permalink raw reply
* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Vlad Yasevich @ 2013-11-15 20:35 UTC (permalink / raw)
To: Chang, Neil Horman
Cc: davem, linux-sctp, netdev, linux-kernel, dreibh, ernstgr
In-Reply-To: <52867D2E.2090907@gmail.com>
On 11/15/2013 02:59 PM, Chang wrote:
> I've tried to catch you guys up~
>
> Here's a quick question:
>
> Where are the default [transport->pf_retrans] and
> [transport->pathmaxrtx] set?
They are initially set through the sctp_net_init. Later
they can be changed through sysctl or via socket options.
> I could figure out that they could be set
> through setsockopt(SCTP_PEER_ADDR_THLDS, ...) (but it seems like the
> SCTP library has not supported such option yet, maybe that's due to my
> library is out of date). So by default both of the two threshold are zero.
pf_retrans starts at 0. pathmaxrtx has always started at 5.
You should be able to use the socket option directly, but you might need
a newer header. You can always add the value to your application as well.
>
> Here's my question: Does it go conflict with "the recommended value for
> Path.Max.Retrans in the standard is 5"?
Path.Max.Retrans is set to 5 (at least on my system...)
-vlad
>
> Thanks!
>
> On 11/15/2013 03:56 PM, Neil Horman wrote:
>> On Fri, Nov 15, 2013 at 09:00:58AM -0500, Vlad Yasevich wrote:
>>> On 11/15/2013 07:30 AM, Neil Horman wrote:
>>>> On Thu, Nov 14, 2013 at 09:34:55PM -0500, Vlad Yasevich wrote:
>>>>> On 11/14/2013 03:40 PM, Chang Xiangzhong wrote:
>>>>>> Expected Behavior:
>>>>>> When hearing an ack from a tranport/path, set its state to
>>>>>> normal/on if it's
>>>>>> in abnormal(__partial_failure__ or inactive) state.
>>>>>>
>>>>>> state machine of tranport->state
>>>>>> Whenever a T3_RTX timer expires, then transport->error_count++.
>>>>>> When (association->pf_retrans < transport->error_count <
>>>>>> tranport->pathmaxrtx)
>>>>>> transport->state = SCTP_PF //partial failure
>>>>>>
>>>>>> When a heartbeat-ack comes or conventional ack acknowledged its
>>>>>> availability,
>>>>>> transport->state = SCTP_ON
>>>>>>
>>>>>> Signed-off-by: Chang Xiangzhong <changxiangzhong@gmail.com>
>>>>>> Fixes: 5aa93bcf66f ("sctp: Implement quick failover draft from
>>>>>> tsvwg")
>>>>> I don't think this is right. The spec states:
>>>>> 8. ACKs for retransmissions do not transition a PF destination back
>>>>> to Active state, since a sender cannot disambiguate whether
>>>>> the
>>>>> ack was for the original transmission or the
>>>>> retransmission(s).
>>>>>
>>>>>
>>>>> Now, the proper way to this would would be modify
>>>>> sctp_assoc_control_transport() to transition the transport state to
>>>>> ACTIVE if it was PF transport that was chosen to send data.
>>>>>
>>>>> -vlad
>>>>>
>>>> I agree, this patch doesn't agree with the spec, the only time we
>>>> transition
>>> >from PF to ACTIVE should be on receipt of ack of new data.
>>>
>>> You mean HB ACK, right? The 02 spec that see on the ietf site doesn't
>>> mention anything about transition on SACKs. Also, there is no way to
>>> tell right now if the ack is for new or retransmitted data. We could
>>> mark chunks that are retransmitted though.
>>>
>> Yes, sorry I wasn't clear, I was speaknig about HB Acks.
>>
>>>> I'm not even sure if
>>>> we should allow PF transports to be selected to send new data.
>>>> Currently a
>>>> potentially failed transport will get ignored when specified, and
>>>> the stack will
>>>> use the active path in its place. Only if all transports are PF
>>>> will a PF
>>>> transport be chosen.
>>> Not even that :(. If all transports are PF, we are going to camp on
>>> the primary path instead of choosing a PF transport with the lowest
>>> error count.
>>>
>> Yes, we don't do the smallest error count check, though I have to
>> wonder if
>> thats really worthwhile. If all your transports are PF, you're a step
>> away from
>> a connection reset anyway.
>> Neil
>>
>>> -vlad
>>>
>>>> Neil
>>>>
>>>>>> ---
>>>>>> net/sctp/outqueue.c | 1 +
>>>>>> 1 file changed, 1 insertion(+)
>>>>>>
>>>>>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>>>>>> index 94df758..2557fa5 100644
>>>>>> --- a/net/sctp/outqueue.c
>>>>>> +++ b/net/sctp/outqueue.c
>>>>>> @@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct
>>>>>> sctp_outq *q,
>>>>>> * active if it is not so marked.
>>>>>> */
>>>>>> if ((transport->state == SCTP_INACTIVE ||
>>>>>> + transport->state == SCTP_PF ||
>>>>>> transport->state == SCTP_UNCONFIRMED) &&
>>>>>> sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
>>>>>> sctp_assoc_control_transport(
>>>>>>
>>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>
^ permalink raw reply
* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Chang @ 2013-11-15 20:38 UTC (permalink / raw)
To: Vlad Yasevich, Neil Horman
Cc: davem, linux-sctp, netdev, linux-kernel, dreibh, ernstgr
In-Reply-To: <52868582.2020509@gmail.com>
Thank you very much Neil and Vlad!
That's exactly what I've been looking for. I'm quite unfamiliar iwth the
sctp_net_init part.
Have a nice weekend!
^ permalink raw reply
* Re: [PATCH] random: seed random_int_secret at least poorly at core_initcall time
From: Theodore Ts'o @ 2013-11-15 21:05 UTC (permalink / raw)
To: Kees Cook
Cc: Daniel Borkmann, David S. Miller,
shemminger-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ, Florian Weimer,
netdev-u79uwXL29TY76Z2rM5mHXA, Eric Dumazet,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAGXu5j+ySEdQBXKkspYC=svfekBja2Z_2tcWSAOEbvyiMLf=aA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Nov 15, 2013 at 10:33:04AM -0800, Kees Cook wrote:
>
> When the static stack canary was mentioned during the ARM summit, I
> dug around a little bit and saw that at very early boot, yes, it was
> always the same, but after boot finished, it was different from boot
> to boot. I didn't get far enough to figure out what was changing it
> later on.
I've been in Seoul this past week, so I haven't had a chance look more
closely at this, but what's not clear to me what the impact will be of
having a static statck canary during early boot. Does that mean the
stack canary for the certain kernel threads and the pid 1 are always
constant, since their stacks are getting set up before we're actually
able to initialize the stack canary with something random?
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2] net: Do not include padding in TCP GRO checksum
From: Eric Dumazet @ 2013-11-15 21:44 UTC (permalink / raw)
To: Alexander Duyck; +Cc: davem, netdev, edumazet, herbert
In-Reply-To: <20131115200046.10597.31528.stgit@ahduyck-fpga.jf.intel.com>
On Fri, 2013-11-15 at 12:03 -0800, Alexander Duyck wrote:
> In some recent tests I found the TCP checksum was being treated as valid
> for certain frames with padding on them. On closer inspection I found the
> issue was that GRO was using the skb->len instead of the length recorded in
> the IP/IPv6 header to determine the number of bytes to checksum. As such
> padded frames that actually had invalid checksums generated by adding the
> padding to the checksum were being incorrectly tagged as valid.
>
> This change corrects that by using the tot_len from IPv4 headers and the
> payload_len from IPv6 headers to compute the total number of bytes to be
> included in the checksum.
>
> To address the fact that skb->csum is invalid when a padded frame is
> received I have updated the code to fall though to the CHECKSUM_NONE path
> for CHECKSUM_COMPLETE frames that contain padding.
>
> Signed-off-by: Alexander Duyck <alexander.h.duyck@intel.com>
> ---
>
> v2: Update byte ordering of tot_len and payload_len so it is in host order.
> Updated CHECKSUM_COMPLETE path so it falls back through CHECKSUM_NONE for
> padded frames since this is how it is handled in ip_rcv.
>
> I have tested and verified the CHECKSUM_NONE path works, but I don't have
> any adapters that generate CHECKSUM_COMPLETE to test with.
>
> net/ipv4/tcp_offload.c | 26 +++++++++++++++++---------
> net/ipv6/tcpv6_offload.c | 27 +++++++++++++++++----------
> 2 files changed, 34 insertions(+), 19 deletions(-)
>
> diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
> index a2b68a1..17b5d14 100644
> --- a/net/ipv4/tcp_offload.c
> +++ b/net/ipv4/tcp_offload.c
> @@ -273,26 +273,34 @@ static int tcp_v4_gso_send_check(struct sk_buff *skb)
> static struct sk_buff **tcp4_gro_receive(struct sk_buff **head, struct sk_buff *skb)
> {
> const struct iphdr *iph = skb_gro_network_header(skb);
> + int length = ntohs(iph->tot_len);
Sorry, but couldn't iph->tot_len be larger than skb_gro_len(skb) ?
It's not validated in inet_gro_complete() ...
skb_checksum(skb, ..., length, wsum) could crash.
^ permalink raw reply
* Re: [PATCH] net: sctp: recover a tranport when an ack comes
From: Chang @ 2013-11-15 22:04 UTC (permalink / raw)
To: Vlad Yasevich, nhorman
Cc: davem, linux-sctp, netdev, linux-kernel, dreibh, ernstgr
In-Reply-To: <5285884F.8030907@gmail.com>
On 11/15/2013 03:34 AM, Vlad Yasevich wrote:
> I don't think this is right. The spec states:
> 8. ACKs for retransmissions do not transition a PF destination back
> to Active state, since a sender cannot disambiguate whether the
> ack was for the original transmission or the retransmission(s).
>
Could you please reconsider my proposal?
In the rule 8, it clearly specifies ACKs for *retransmission* do not
transition ... But those chunks were not retransmitted!
Every transport maintains its own [transport->transmitted] queue, when
retransmit happens (no matter time_out or fast_retransmit). The chunk
would be removed from the queue and moved to the sctp_outq->retransmit.
static void sctp_check_transmitted(...) {
...
if (transport) { /*<=======This proves that its not the
outq->retransmit (the retransmitted queue)*/
if (bytes_acked) {
...
if((transport->state in [INACTIVE, UNCONFIRMED, PF]...)
sctp_assoc_control_transport(..., SCTP_TRANSPORT_UP).
In addition, if its not appropriate to transition PF->ACTIVE, why is it
appropriate to transition INACTIVE->ACTIVE (the original implementation).
>
> Now, the proper way to this would would be modify
> sctp_assoc_control_transport() to transition the transport state to
> ACTIVE if it was PF transport that was chosen to send data.
>
> -vlad
>
>> ---
>> net/sctp/outqueue.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
>> index 94df758..2557fa5 100644
>> --- a/net/sctp/outqueue.c
>> +++ b/net/sctp/outqueue.c
>> @@ -1517,6 +1517,7 @@ static void sctp_check_transmitted(struct
>> sctp_outq *q,
>> * active if it is not so marked.
>> */
>> if ((transport->state == SCTP_INACTIVE ||
>> + transport->state == SCTP_PF ||
>> transport->state == SCTP_UNCONFIRMED) &&
>> sctp_cmp_addr_exact(&transport->ipaddr, saddr)) {
>> sctp_assoc_control_transport(
>>
>
^ permalink raw reply
* Re: [OOPS][3.12] BUG: unable to handle kernel NULL pointer dereference at 0000000c
From: Bjorn Helgaas @ 2013-11-15 22:29 UTC (permalink / raw)
To: Ian Kumlien
Cc: linux-kernel@vger.kernel.org, Eric Dumazet, netdev,
David S. Miller, Alexander Aring
In-Reply-To: <20131115183422.GA9417@pomac.netswarm.net>
[+cc David, Eric, Alex, netdev]
Alex reported a similar issue at
http://marc.info/?l=linux-netdev&m=138355719901790&w=4
On Fri, Nov 15, 2013 at 11:34 AM, Ian Kumlien <pomac@vapor.com> wrote:
> Hi,
>
> After a lot of wondering i finally tracked down the bug that was hitting
> me since 3.12-rc7. Since this is a firewall I haven't actually noticed
> it all the time. But when i saw that it rebooted too often, i enabled
> netconsole and this is the output:
>
> BUG: unable to handle kernel NULL pointer dereference at 0000000c
> IP: [<c18196db>] _decode_session6+0x8b/0x370
> *pde = 00000000
> Oops: 0000 [#1] SMP
> Modules linked in: netconsole tun
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 3.12.0 #55
> Hardware name: MICRO-STAR INTERNATIONAL CO., LTD MS-9632/MS-9632, BIOS 6.00 PG 05/16/2007
> task: c1b64880 ti: f600a000 task.ti: c1b5a000
> EIP: 0060:[<c18196db>] EFLAGS: 00210202 CPU: 0
> EIP is at _decode_session6+0x8b/0x370
> EAX: 00000000 EBX: f2c42c00 ECX: 00000001 EDX: e351a0a2
> ESI: 00000000 EDI: f600be70 EBP: f600be34 ESP: f600bdfc
> DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
> CR0: 8005003b CR2: 0000000c CR3: 235e8000 CR4: 000007d0
> Stack:
> f600be30 00282c00 00000001 c1bb24e0 f63f8000 c1baa780 f2c42c00 c17d653f
> f2c42c00 c1807178 00000001 00000000 e3791f00 e3791f00 00000000 00000000
> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> Call Trace:
> [<c17d653f>] ? __xfrm_decode_session+0x1f/0x30
> [<c1807178>] ? icmpv6_route_lookup+0xa8/0x170
> [<c1807693>] ? icmp6_send+0x453/0x6e0
> [<c177dd7c>] ? ip_local_deliver_finish+0x7c/0x1f0
> [<c177dd00>] ? ip_rcv_finish+0x310/0x310
> [<c177db03>] ? ip_rcv_finish+0x113/0x310
> [<c1807240>] ? icmpv6_route_lookup+0x170/0x170
> [<c182dc64>] ? icmpv6_send+0x24/0x30
> [<c180df2f>] ? ip6_expire_frag_queue+0x16f/0x180
> [<c1823390>] ? nf_ct_net_init+0x60/0x60
> [<c1075efc>] ? call_timer_fn.isra.27+0x1c/0x80
> [<c155ff1b>] ? e1000e_poll+0x13b/0x2e0
> [<c1823390>] ? nf_ct_net_init+0x60/0x60
> [<c1076094>] ? run_timer_softirq+0x134/0x1d0
> [<c1071255>] ? __do_softirq+0xa5/0x160
> [<c10711b0>] ? remote_softirq_cpu_notify+0xa0/0xa0
> <IRQ>
> [<c1071416>] ? irq_exit+0x66/0x90
> [<c105dff5>] ? smp_apic_timer_interrupt+0x35/0x50
> [<c187196d>] ? apic_timer_interrupt+0x2d/0x34
> [<c103d8d2>] ? default_idle+0x2/0x10
> [<c103df26>] ? arch_cpu_idle+0x16/0x20
> [<c10a1ed9>] ? cpu_startup_entry+0x49/0x130
> [<c1bc4948>] ? start_kernel+0x29e/0x2a3
> [<c1bc44ef>] ? repair_env_string+0x4d/0x4d
> Code: 00 00 f3 ab 74 08 66 c7 07 00 00 83 c7 02 83 e6 01 74 03 c6 07 00 8b 83 90 00 00 00 8b 4c 24 08 89 45 08 8b 43 48 83 e0 fe 85 c9 <8b> 40 0c 8b 80 88 00 00 00 89 45 00 0f 84 1b 01 00 00 8b 42 08
> EIP: [<c18196db>] _decode_session6+0x8b/0x370 SS:ESP 0068:f600bdfc
> CR2: 000000000000000c
> ---[ end trace 0cbf7fb6e6aa1f45 ]---
> Kernel panic - not syncing: Fatal exception in interrupt
> ---
>
> Any clue besides just disabling ipv6? ;)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH v2.50 2/6] ofp-actions: Account for OF version when enforcing action consistency
From: Ben Pfaff @ 2013-11-15 22:34 UTC (permalink / raw)
To: Simon Horman
Cc: dev, netdev, Jesse Gross, Pravin B Shelar, Ravi K, Joe Stringer
In-Reply-To: <1384506623-18503-3-git-send-email-horms@verge.net.au>
On Fri, Nov 15, 2013 at 06:10:19PM +0900, Simon Horman wrote:
> The aim of this patch is to provide infrastructure to differentiate
> between OpenFlow1.1 - 1.2 and OpenFlow1.3+ consistency checking. It
> also provides enhanced OpenFlow1.0 consistency checking.
>
> Some different versions of OpenFlow require different consistency
> checking.
>
> 1. OpenFlow1.0
>
> This variant implicitly pushes a VLAN tag if one doesn't exist
> and an action to modify a VLAN tag is encountered.
>
> MPLS push is supported as a Nicira extension whose behaviour is
> the same as OpenFlow1.1 - 1.2.
>
> In practice Open vSwitch allows inconsistent OpenFlow 1.0 actions so
> this portion of the change is just for completeness. I do not believe it
> has any run-time affect. And I would not be opposed to folding this case
> into the handling of OpenFlow1.1 - 1.2.
>
> 2. OpenFlow1.1 - 1.2
>
> This does not have the implicit VLAN tag push behaviour of OpenFlow1.0.
>
> An MPLS push action pushes an MPLS LSE after any VLAN tags that are
> present. This is pre-OpenFlow1.3 tag ordering.
>
> 3. OpenFlow1.3+
>
> This does not have the implicit VLAN tag push behaviour of OpenFlow1.0.
>
> An MPLS push action pushes an MPLS LSE before any VLAN tags that are
> present. This is OpenFlow1.3+ tag ordering. Its implication
> is that after an MPLS push action a packet is no longer a VLAN
> packet as there are no longer any VLAN tags immediately after
> the ethernet header: there is now an MPLS LSE there.
>
> Currently Open vSwitch does not implement OpenFlow1.3+ tag ordering
> so this patch uses the same consistency checking for OpenFlow1.3+
> as for OpenFlow1.1 - 1.2.
>
> A subsequent patch, which implements OpenFlow1.3+ tag ordering,
> makes use of the infrastructure provided by this patch to check
> actions accordingly.
>
> Unfortunately ofpact_get_SET_VLAN_VID(a)->push_vlan_if_needed and similar
> devices can't be used in order to implement logic as they are not set when
> when ofpact_check__() is called via parse_ofp_str().
>
> Instead this patch takes the approach of adding an OpenFlow version
> parameter to ofpact_check__().
>
> A new function ofpact_check_usable_protocols() allows trimming the
> usable_protocols based on action checking.
>
> The net effect of this change on run-time is only to increase logging under
> some circumstances as ofpacts_check() may now be called up to four times
> instead of up to twice for each invocation of parse_ofp_str__()
>
> Signed-off-by: Simon Horman <horms@verge.net.au>
I think I have a better way. I posted patches:
http://openvswitch.org/pipermail/dev/2013-November/033968.html
http://openvswitch.org/pipermail/dev/2013-November/033967.html
^ permalink raw reply
* Re: [PATCH net v3 1/4] r8152: fix tx/rx memory overflow
From: David Miller @ 2013-11-15 22:39 UTC (permalink / raw)
To: hayeswang; +Cc: netdev, nic_swsd, linux-kernel, linux-usb
In-Reply-To: <1384502279-9959-2-git-send-email-hayeswang@realtek.com>
From: Hayes Wang <hayeswang@realtek.com>
Date: Fri, 15 Nov 2013 15:57:56 +0800
> + unsigned pkt_len;
Please fully specify the type as "unsigned int". Please check for this
problem in the rest of your patches too.
Thanks.
^ permalink raw reply
* Re: [OOPS][3.12] BUG: unable to handle kernel NULL pointer dereference at 0000000c
From: David Miller @ 2013-11-15 22:44 UTC (permalink / raw)
To: bhelgaas; +Cc: pomac, linux-kernel, edumazet, netdev, alex.aring
In-Reply-To: <CAErSpo4jh7pO=jhZTvHD+sT2O12Ly6V3e_oiac8gHqmdmqevkg@mail.gmail.com>
From: Bjorn Helgaas <bhelgaas@google.com>
Date: Fri, 15 Nov 2013 15:29:53 -0700
> [+cc David, Eric, Alex, netdev]
>
> Alex reported a similar issue at
> http://marc.info/?l=linux-netdev&m=138355719901790&w=4
Fixed by:
commit 84502b5ef9849a9694673b15c31bd3ac693010ae
Author: Steffen Klassert <steffen.klassert@secunet.com>
Date: Wed Oct 30 11:16:28 2013 +0100
xfrm: Fix null pointer dereference when decoding sessions
On some codepaths the skb does not have a dst entry
when xfrm_decode_session() is called. So check for
a valid skb_dst() before dereferencing the device
interface index. We use 0 as the device index if
there is no valid skb_dst(), or at reverse decoding
we use skb_iif as device interface index.
Bug was introduced with git commit bafd4bd4dc
("xfrm: Decode sessions with output interface.").
Reported-by: Meelis Roos <mroos@linux.ee>
Tested-by: Meelis Roos <mroos@linux.ee>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index 4764ee4..e1a6393 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -104,10 +104,14 @@ _decode_session4(struct sk_buff *skb, struct flowi *fl, int reverse)
const struct iphdr *iph = ip_hdr(skb);
u8 *xprth = skb_network_header(skb) + iph->ihl * 4;
struct flowi4 *fl4 = &fl->u.ip4;
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl4, 0, sizeof(struct flowi4));
fl4->flowi4_mark = skb->mark;
- fl4->flowi4_oif = skb_dst(skb)->dev->ifindex;
+ fl4->flowi4_oif = reverse ? skb->skb_iif : oif;
if (!ip_is_fragment(iph)) {
switch (iph->protocol) {
diff --git a/net/ipv6/xfrm6_policy.c b/net/ipv6/xfrm6_policy.c
index dd503a3..5f8e128 100644
--- a/net/ipv6/xfrm6_policy.c
+++ b/net/ipv6/xfrm6_policy.c
@@ -135,10 +135,14 @@ _decode_session6(struct sk_buff *skb, struct flowi *fl, int reverse)
struct ipv6_opt_hdr *exthdr;
const unsigned char *nh = skb_network_header(skb);
u8 nexthdr = nh[IP6CB(skb)->nhoff];
+ int oif = 0;
+
+ if (skb_dst(skb))
+ oif = skb_dst(skb)->dev->ifindex;
memset(fl6, 0, sizeof(struct flowi6));
fl6->flowi6_mark = skb->mark;
- fl6->flowi6_oif = skb_dst(skb)->dev->ifindex;
+ fl6->flowi6_oif = reverse ? skb->skb_iif : oif;
fl6->daddr = reverse ? hdr->saddr : hdr->daddr;
fl6->saddr = reverse ? hdr->daddr : hdr->saddr;
^ 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