* Re: [PATCH v6 03/21] libnvdimm: control character device and nvdimm_bus sysfs attributes
From: Rafael J. Wysocki @ 2015-06-17 22:00 UTC (permalink / raw)
To: Dan Williams
Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, mingo-DgEjT+Ai2ygdnm+yROfE0A,
sfr-3FnU+UHB4dNDw9hX6IcOSA, rafael-DgEjT+Ai2ygdnm+yROfE0A,
neilb-l3A5Bk7waGM, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
Rafael J. Wysocki, Robert Moore,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA, jmoyer-H+wXaHxf7aLQT0dZR+AlfA,
axboe-tSWWG44O7X1aa/9Udqfwiw, linux-api-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, hch-jcswGhMUV9g
In-Reply-To: <20150611201024.25175.7686.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Thursday, June 11, 2015 04:10:24 PM Dan Williams wrote:
> The control device for a nvdimm_bus is registered as an "nd" class
> device. The expectation is that there will usually only be one "nd" bus
> registered under /sys/class/nd. However, we allow for the possibility
> of multiple buses and they will listed in discovery order as
> ndctl0...ndctlN. This character device hosts the ioctl for passing
> control messages. The initial command set has a 1:1 correlation with
> the commands listed in the by the "NFIT DSM Example" document [1], but
> this scheme is extensible to future command sets.
>
> Note, nd_ioctl() and the backing ->ndctl() implementation are defined in
> a subsequent patch. This is simply the initial registrations and sysfs
> attributes.
>
> [1]: http://pmem.io/documents/NVDIMM_DSM_Interface_Example.pdf
>
> Cc: Neil Brown <neilb-l3A5Bk7waGM@public.gmane.org>
> Cc: Greg KH <gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>
> Cc: <linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Cc: Robert Moore <robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
For the ACPI part:
Acked-by: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
^ permalink raw reply
* Re: [PATCH v6 02/21] libnvdimm, nfit: initial libnvdimm infrastructure and NFIT support
From: Rafael J. Wysocki @ 2015-06-17 21:59 UTC (permalink / raw)
To: Dan Williams
Cc: linux-nvdimm, mingo, sfr, rafael, neilb, gregkh,
Rafael J. Wysocki, linux-kernel, Robert Moore, linux-acpi, jmoyer,
Lv Zheng, linux-api, akpm, axboe, hch
In-Reply-To: <20150611201018.25175.78715.stgit@dwillia2-desk3.amr.corp.intel.com>
On Thursday, June 11, 2015 04:10:18 PM Dan Williams wrote:
> A struct nvdimm_bus is the anchor device for registering nvdimm
> resources and interfaces, for example, a character control device,
> nvdimm devices, and I/O region devices. The ACPI NFIT (NVDIMM Firmware
> Interface Table) is one possible platform description for such
> non-volatile memory resources in a system. The nfit.ko driver attaches
> to the "ACPI0012" device that indicates the presence of the NFIT and
> parses the table to register a struct nvdimm_bus instance.
>
> Cc: <linux-acpi@vger.kernel.org>
> Cc: Lv Zheng <lv.zheng@intel.com>
> Cc: Robert Moore <robert.moore@intel.com>
> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Acked-by: Jeff Moyer <jmoyer@redhat.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
[cut]
> +
> +static struct acpi_driver acpi_nfit_driver = {
> + .name = KBUILD_MODNAME,
> + .ids = acpi_nfit_ids,
> + .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
But this flag is not needed for now IMO.
> + .ops = {
> + .add = acpi_nfit_add,
> + .remove = acpi_nfit_remove,
> + },
> +};
> +
^ permalink raw reply
* Re: [PATCH v6 02/21] libnvdimm, nfit: initial libnvdimm infrastructure and NFIT support
From: Dan Williams @ 2015-06-17 21:35 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw@public.gmane.org, Ingo Molnar,
Stephen Rothwell, Rafael J. Wysocki, Neil Brown, Greg KH,
Rafael J. Wysocki,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Robert Moore, Linux ACPI, jmoyer, Lv Zheng,
linux-api-u79uwXL29TY76Z2rM5mHXA, Andrew Morton, Jens Axboe,
Christoph Hellwig
In-Reply-To: <1863642.qegEO3HtMW-sKB8Sp2ER+y1GS7QM15AGw@public.gmane.org>
On Wed, Jun 17, 2015 at 2:59 PM, Rafael J. Wysocki <rjw-LthD3rsA81gm4RdzfppkhA@public.gmane.org> wrote:
> On Thursday, June 11, 2015 04:10:18 PM Dan Williams wrote:
>> A struct nvdimm_bus is the anchor device for registering nvdimm
>> resources and interfaces, for example, a character control device,
>> nvdimm devices, and I/O region devices. The ACPI NFIT (NVDIMM Firmware
>> Interface Table) is one possible platform description for such
>> non-volatile memory resources in a system. The nfit.ko driver attaches
>> to the "ACPI0012" device that indicates the presence of the NFIT and
>> parses the table to register a struct nvdimm_bus instance.
>>
>> Cc: <linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
>> Cc: Lv Zheng <lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Cc: Robert Moore <robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Cc: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Acked-by: Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
Thanks Rafael!
> [cut]
>
>> +
>> +static struct acpi_driver acpi_nfit_driver = {
>> + .name = KBUILD_MODNAME,
>> + .ids = acpi_nfit_ids,
>> + .flags = ACPI_DRIVER_ALL_NOTIFY_EVENTS,
>
> But this flag is not needed for now IMO.
Indeed, I'll fold the removal in to a v7 when I add the acks and a few
other straggling bug fixes.
^ permalink raw reply
* [PATCH net-next 3/3] ipv4: ICMP packet inspection for multipath
From: Peter Nørlund @ 2015-06-17 20:08 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-api, Peter Nørlund
In-Reply-To: <1434571686-5149-1-git-send-email-pch@ordbogen.com>
ICMP packets are inspected to let them route together with the flow they
belong to, allowing anycast environments to work with ECMP.
Signed-off-by: Peter Nørlund <pch@ordbogen.com>
---
net/ipv4/icmp.c | 27 ++++++++++++++++++-
net/ipv4/route.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++----------
2 files changed, 92 insertions(+), 15 deletions(-)
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 3abcfea..20f1d5e 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -447,6 +447,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
{
struct rtable *rt, *rt2;
struct flowi4 fl4_dec;
+ struct flowi4 mp_flow;
int err;
memset(fl4, 0, sizeof(*fl4));
@@ -459,7 +460,31 @@ static struct rtable *icmp_route_lookup(struct net *net,
fl4->fl4_icmp_type = type;
fl4->fl4_icmp_code = code;
security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
- rt = __ip_route_output_key(net, fl4, NULL);
+
+ /* Source and destination is swapped. See ip_multipath_flow */
+ mp_flow.saddr = iph->daddr;
+ mp_flow.daddr = iph->saddr;
+ mp_flow.flowi4_proto = iph->protocol;
+ mp_flow.fl4_sport = 0;
+ mp_flow.fl4_dport = 0;
+ if (!ip_is_fragment(iph)) {
+ if (iph->protocol == IPPROTO_TCP ||
+ iph->protocol == IPPROTO_UDP ||
+ iph->protocol == IPPROTO_SCTP) {
+ __be16 _ports[2];
+ const __be16 *ports;
+
+ ports = skb_header_pointer(skb_in, iph->ihl * 4,
+ sizeof(_ports),
+ &_ports);
+ if (ports) {
+ mp_flow.fl4_sport = ports[1];
+ mp_flow.fl4_dport = ports[0];
+ }
+ }
+ }
+
+ rt = __ip_route_output_key(net, fl4, &mp_flow);
if (IS_ERR(rt))
return rt;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index a1ec62c..bab4318 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1635,31 +1635,83 @@ out:
/* Fill flow key data based on packet for use in multipath routing. */
static void ip_multipath_flow(const struct sk_buff *skb, struct flowi4 *flow)
{
- const struct iphdr *iph;
-
- iph = ip_hdr(skb);
-
- flow->saddr = iph->saddr;
- flow->daddr = iph->daddr;
- flow->flowi4_proto = iph->protocol;
+ struct icmphdr _icmph;
+ struct iphdr _inner_iph;
+ const struct iphdr *outer_iph;
+ const struct icmphdr *icmph;
+ const struct iphdr *inner_iph;
+ unsigned int offset;
+ __be16 _ports[2];
+ const __be16 *ports;
+
+ outer_iph = ip_hdr(skb);
+
+ flow->saddr = outer_iph->saddr;
+ flow->daddr = outer_iph->daddr;
+ flow->flowi4_proto = outer_iph->protocol;
flow->fl4_sport = 0;
flow->fl4_dport = 0;
- if (unlikely(ip_is_fragment(iph)))
+ if (unlikely(ip_is_fragment(outer_iph)))
return;
- if (iph->protocol == IPPROTO_TCP ||
- iph->protocol == IPPROTO_UDP ||
- iph->protocol == IPPROTO_SCTP) {
- __be16 _ports;
- const __be16 *ports;
+ offset = outer_iph->ihl * 4;
- ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
+ if (outer_iph->protocol == IPPROTO_TCP ||
+ outer_iph->protocol == IPPROTO_UDP ||
+ outer_iph->protocol == IPPROTO_SCTP) {
+ ports = skb_header_pointer(skb, offset, sizeof(_ports),
&_ports);
if (ports) {
flow->fl4_sport = ports[0];
flow->fl4_dport = ports[1];
}
+
+ return;
+ }
+
+ if (outer_iph->protocol != IPPROTO_ICMP)
+ return;
+
+ icmph = skb_header_pointer(skb, offset, sizeof(_icmph), &_icmph);
+ if (!icmph)
+ return;
+
+ if (icmph->type != ICMP_DEST_UNREACH &&
+ icmph->type != ICMP_SOURCE_QUENCH &&
+ icmph->type != ICMP_REDIRECT &&
+ icmph->type != ICMP_TIME_EXCEEDED &&
+ icmph->type != ICMP_PARAMETERPROB) {
+ return;
+ }
+
+ offset += sizeof(_icmph);
+ inner_iph = skb_header_pointer(skb, offset, sizeof(_inner_iph),
+ &_inner_iph);
+ if (inner_iph)
+ return;
+
+ /* Since the ICMP payload contains a packet sent from the current
+ * recipient, we swap source and destination addresses and ports
+ */
+ flow->saddr = inner_iph->daddr;
+ flow->daddr = inner_iph->saddr;
+ flow->flowi4_proto = inner_iph->protocol;
+
+ if (unlikely(ip_is_fragment(inner_iph)))
+ return;
+
+ if (inner_iph->protocol != IPPROTO_TCP &&
+ inner_iph->protocol != IPPROTO_UDP &&
+ inner_iph->protocol != IPPROTO_SCTP) {
+ return;
+ }
+
+ offset += inner_iph->ihl * 4;
+ ports = skb_header_pointer(skb, offset, sizeof(_ports), &_ports);
+ if (ports) {
+ flow->fl4_sport = ports[1];
+ flow->fl4_dport = ports[0];
}
}
#endif /* CONFIG_IP_ROUTE_MULTIPATH */
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/3] ipv4: L3 and L4 hash-based multipath routing
From: Peter Nørlund @ 2015-06-17 20:08 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-api, Peter Nørlund
In-Reply-To: <1434571686-5149-1-git-send-email-pch@ordbogen.com>
This patch adds L3 and L4 hash-based multipath routing, selectable on a
per-route basis with the reintroduced RTA_MP_ALGO attribute. The default is
now RT_MP_ALG_L3_HASH.
Signed-off-by: Peter Nørlund <pch@ordbogen.com>
---
include/net/ip_fib.h | 4 ++-
include/net/route.h | 5 ++--
include/uapi/linux/rtnetlink.h | 14 ++++++++++-
net/ipv4/fib_frontend.c | 4 +++
net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++---
net/ipv4/icmp.c | 4 +--
net/ipv4/route.c | 56 +++++++++++++++++++++++++++++++++++-------
net/ipv4/xfrm4_policy.c | 2 +-
8 files changed, 103 insertions(+), 20 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 4be4f25..250d98e 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -37,6 +37,7 @@ struct fib_config {
u32 fc_flags;
u32 fc_priority;
__be32 fc_prefsrc;
+ int fc_mp_alg;
struct nlattr *fc_mx;
struct rtnexthop *fc_mp;
int fc_mx_len;
@@ -116,6 +117,7 @@ struct fib_info {
int fib_nhs;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_mp_weight;
+ int fib_mp_alg;
#endif
struct rcu_head rcu;
struct fib_nh fib_nh[0];
@@ -308,7 +310,7 @@ int ip_fib_check_default(__be32 gw, struct net_device *dev);
int fib_sync_down_dev(struct net_device *dev, int force);
int fib_sync_down_addr(struct net *net, __be32 local);
int fib_sync_up(struct net_device *dev);
-void fib_select_multipath(struct fib_result *res);
+void fib_select_multipath(struct fib_result *res, const struct flowi4 *flow);
/* Exported by fib_trie.c */
void fib_trie_init(void);
diff --git a/include/net/route.h b/include/net/route.h
index fe22d03..1fc7deb 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -110,7 +110,8 @@ struct in_device;
int ip_rt_init(void);
void rt_cache_flush(struct net *net);
void rt_flush_dev(struct net_device *dev);
-struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp);
+struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp,
+ const struct flowi4 *mp_flow);
struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
struct sock *sk);
struct dst_entry *ipv4_blackhole_route(struct net *net,
@@ -267,7 +268,7 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
sport, dport, sk);
if (!dst || !src) {
- rt = __ip_route_output_key(net, fl4);
+ rt = __ip_route_output_key(net, fl4, NULL);
if (IS_ERR(rt))
return rt;
ip_rt_put(rt);
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 17fb02f..dff4a72 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -271,6 +271,18 @@ enum rt_scope_t {
#define RTM_F_EQUALIZE 0x400 /* Multipath equalizer: NI */
#define RTM_F_PREFIX 0x800 /* Prefix addresses */
+/* Multipath algorithms */
+
+enum rt_mp_alg_t {
+ RT_MP_ALG_L3_HASH, /* Was IP_MP_ALG_NONE */
+ RT_MP_ALG_PER_PACKET, /* Was IP_MP_ALG_RR */
+ RT_MP_ALG_DRR, /* not used */
+ RT_MP_ALG_RANDOM, /* not used */
+ RT_MP_ALG_WRANDOM, /* not used */
+ RT_MP_ALG_L4_HASH,
+ __RT_MP_ALG_MAX
+};
+
/* Reserved table identifiers */
enum rt_class_t {
@@ -301,7 +313,7 @@ enum rtattr_type_t {
RTA_FLOW,
RTA_CACHEINFO,
RTA_SESSION, /* no longer used */
- RTA_MP_ALGO, /* no longer used */
+ RTA_MP_ALGO,
RTA_TABLE,
RTA_MARK,
RTA_MFC_STATS,
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 872494e..376e8c1 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -590,6 +590,7 @@ const struct nla_policy rtm_ipv4_policy[RTA_MAX + 1] = {
[RTA_PREFSRC] = { .type = NLA_U32 },
[RTA_METRICS] = { .type = NLA_NESTED },
[RTA_MULTIPATH] = { .len = sizeof(struct rtnexthop) },
+ [RTA_MP_ALGO] = { .type = NLA_U32 },
[RTA_FLOW] = { .type = NLA_U32 },
};
@@ -650,6 +651,9 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
cfg->fc_mp = nla_data(attr);
cfg->fc_mp_len = nla_len(attr);
break;
+ case RTA_MP_ALGO:
+ cfg->fc_mp_alg = nla_get_u32(attr);
+ break;
case RTA_FLOW:
cfg->fc_flow = nla_get_u32(attr);
break;
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 8c8df80..da06e88 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -257,6 +257,11 @@ static inline int nh_comp(const struct fib_info *fi, const struct fib_info *ofi)
{
const struct fib_nh *onh = ofi->fib_nh;
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+ if (fi->fib_mp_alg != ofi->fib_mp_alg)
+ return -1;
+#endif
+
for_nexthops(fi) {
if (nh->nh_oif != onh->nh_oif ||
nh->nh_gw != onh->nh_gw ||
@@ -896,6 +901,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
if (cfg->fc_mp) {
#ifdef CONFIG_IP_ROUTE_MULTIPATH
+ fi->fib_mp_alg = cfg->fc_mp_alg;
err = fib_get_nhs(fi, cfg->fc_mp, cfg->fc_mp_len, cfg);
if (err != 0)
goto failure;
@@ -1085,6 +1091,10 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
struct rtnexthop *rtnh;
struct nlattr *mp;
+ if (fi->fib_mp_alg &&
+ nla_put_u32(skb, RTA_MP_ALGO, fi->fib_mp_alg))
+ goto nla_put_failure;
+
mp = nla_nest_start(skb, RTA_MULTIPATH);
if (!mp)
goto nla_put_failure;
@@ -1312,15 +1322,31 @@ int fib_sync_up(struct net_device *dev)
}
/*
- * The algorithm is suboptimal, but it provides really
- * fair weighted route distribution.
+ * Compute multipath hash based on 3- or 5-tuple
*/
-void fib_select_multipath(struct fib_result *res)
+static int fib_multipath_hash(const struct fib_result *res,
+ const struct flowi4 *flow)
+{
+ u32 hash = flow->saddr ^ flow->daddr;
+
+ if (res->fi->fib_mp_alg == RT_MP_ALG_L4_HASH && flow->flowi4_proto != 0)
+ hash ^= flow->flowi4_proto ^ flow->fl4_sport ^ flow->fl4_dport;
+
+ hash ^= hash >> 16;
+ hash ^= hash >> 8;
+ return hash & 0xFF;
+}
+
+void fib_select_multipath(struct fib_result *res, const struct flowi4 *flow)
{
struct fib_info *fi = res->fi;
u8 w;
- w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
+ if (res->fi->fib_mp_alg == RT_MP_ALG_PER_PACKET) {
+ w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
+ } else {
+ w = fib_multipath_hash(res, flow);
+ }
for_nexthops(fi) {
if (w >= atomic_read(&nh->nh_mp_upper_bound))
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index f5203fb..3abcfea 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -459,7 +459,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
fl4->fl4_icmp_type = type;
fl4->fl4_icmp_code = code;
security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
- rt = __ip_route_output_key(net, fl4);
+ rt = __ip_route_output_key(net, fl4, NULL);
if (IS_ERR(rt))
return rt;
@@ -481,7 +481,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
goto relookup_failed;
if (inet_addr_type(net, fl4_dec.saddr) == RTN_LOCAL) {
- rt2 = __ip_route_output_key(net, &fl4_dec);
+ rt2 = __ip_route_output_key(net, &fl4_dec, NULL);
if (IS_ERR(rt2))
err = PTR_ERR(rt2);
} else {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f605598..a1ec62c 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1006,7 +1006,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu,
__build_flow_key(&fl4, NULL, iph, oif,
RT_TOS(iph->tos), protocol, mark, flow_flags);
- rt = __ip_route_output_key(net, &fl4);
+ rt = __ip_route_output_key(net, &fl4, NULL);
if (!IS_ERR(rt)) {
__ip_rt_update_pmtu(rt, &fl4, mtu);
ip_rt_put(rt);
@@ -1025,7 +1025,7 @@ static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
if (!fl4.flowi4_mark)
fl4.flowi4_mark = IP4_REPLY_MARK(sock_net(sk), skb->mark);
- rt = __ip_route_output_key(sock_net(sk), &fl4);
+ rt = __ip_route_output_key(sock_net(sk), &fl4, NULL);
if (!IS_ERR(rt)) {
__ip_rt_update_pmtu(rt, &fl4, mtu);
ip_rt_put(rt);
@@ -1094,7 +1094,7 @@ void ipv4_redirect(struct sk_buff *skb, struct net *net,
__build_flow_key(&fl4, NULL, iph, oif,
RT_TOS(iph->tos), protocol, mark, flow_flags);
- rt = __ip_route_output_key(net, &fl4);
+ rt = __ip_route_output_key(net, &fl4, NULL);
if (!IS_ERR(rt)) {
__ip_do_redirect(rt, skb, &fl4, false);
ip_rt_put(rt);
@@ -1109,7 +1109,7 @@ void ipv4_sk_redirect(struct sk_buff *skb, struct sock *sk)
struct rtable *rt;
__build_flow_key(&fl4, sk, iph, 0, 0, 0, 0, 0);
- rt = __ip_route_output_key(sock_net(sk), &fl4);
+ rt = __ip_route_output_key(sock_net(sk), &fl4, NULL);
if (!IS_ERR(rt)) {
__ip_do_redirect(rt, skb, &fl4, false);
ip_rt_put(rt);
@@ -1631,6 +1631,39 @@ out:
return err;
}
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+/* Fill flow key data based on packet for use in multipath routing. */
+static void ip_multipath_flow(const struct sk_buff *skb, struct flowi4 *flow)
+{
+ const struct iphdr *iph;
+
+ iph = ip_hdr(skb);
+
+ flow->saddr = iph->saddr;
+ flow->daddr = iph->daddr;
+ flow->flowi4_proto = iph->protocol;
+ flow->fl4_sport = 0;
+ flow->fl4_dport = 0;
+
+ if (unlikely(ip_is_fragment(iph)))
+ return;
+
+ if (iph->protocol == IPPROTO_TCP ||
+ iph->protocol == IPPROTO_UDP ||
+ iph->protocol == IPPROTO_SCTP) {
+ __be16 _ports;
+ const __be16 *ports;
+
+ ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
+ &_ports);
+ if (ports) {
+ flow->fl4_sport = ports[0];
+ flow->fl4_dport = ports[1];
+ }
+ }
+}
+#endif /* CONFIG_IP_ROUTE_MULTIPATH */
+
static int ip_mkroute_input(struct sk_buff *skb,
struct fib_result *res,
const struct flowi4 *fl4,
@@ -1638,8 +1671,12 @@ static int ip_mkroute_input(struct sk_buff *skb,
__be32 daddr, __be32 saddr, u32 tos)
{
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- if (res->fi && res->fi->fib_nhs > 1)
- fib_select_multipath(res);
+ if (res->fi && res->fi->fib_nhs > 1) {
+ struct flowi4 mp_flow;
+
+ ip_multipath_flow(skb, &mp_flow);
+ fib_select_multipath(res, &mp_flow);
+ }
#endif
/* create a routing cache entry */
@@ -2012,7 +2049,8 @@ add:
* Major route resolver routine.
*/
-struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
+struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4,
+ const struct flowi4 *mp_flow)
{
struct net_device *dev_out = NULL;
__u8 tos = RT_FL_TOS(fl4);
@@ -2170,7 +2208,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
#ifdef CONFIG_IP_ROUTE_MULTIPATH
if (res.fi->fib_nhs > 1 && fl4->flowi4_oif == 0)
- fib_select_multipath(&res);
+ fib_select_multipath(&res, (mp_flow ? mp_flow : fl4));
else
#endif
if (!res.prefixlen &&
@@ -2273,7 +2311,7 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
struct sock *sk)
{
- struct rtable *rt = __ip_route_output_key(net, flp4);
+ struct rtable *rt = __ip_route_output_key(net, flp4, NULL);
if (IS_ERR(rt))
return rt;
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index bff6974..7eae158 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -31,7 +31,7 @@ static struct dst_entry *__xfrm4_dst_lookup(struct net *net, struct flowi4 *fl4,
if (saddr)
fl4->saddr = saddr->a4;
- rt = __ip_route_output_key(net, fl4);
+ rt = __ip_route_output_key(net, fl4, NULL);
if (!IS_ERR(rt))
return &rt->dst;
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 1/3] ipv4: Lock-less per-packet multipath
From: Peter Nørlund @ 2015-06-17 20:08 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
linux-api-u79uwXL29TY76Z2rM5mHXA, Peter Nørlund
In-Reply-To: <1434571686-5149-1-git-send-email-pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
The current multipath attempted to be quasi random, but in most cases it
behaved just like a round robin balancing. This patch refactors the
algorithm to be exactly that and in doing so, avoids the spin lock.
The new design paves the way for hash-based multipath, replacing the
modulo with thresholds, minimizing disruption in case of failing paths or
route replacements.
Signed-off-by: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
---
include/net/ip_fib.h | 6 +--
net/ipv4/Kconfig | 1 +
net/ipv4/fib_semantics.c | 116 ++++++++++++++++++++++++++---------------------
3 files changed, 68 insertions(+), 55 deletions(-)
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 54271ed..4be4f25 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -76,8 +76,8 @@ struct fib_nh {
unsigned int nh_flags;
unsigned char nh_scope;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- int nh_weight;
- int nh_power;
+ int nh_mp_weight;
+ atomic_t nh_mp_upper_bound;
#endif
#ifdef CONFIG_IP_ROUTE_CLASSID
__u32 nh_tclassid;
@@ -115,7 +115,7 @@ struct fib_info {
#define fib_advmss fib_metrics[RTAX_ADVMSS-1]
int fib_nhs;
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- int fib_power;
+ int fib_mp_weight;
#endif
struct rcu_head rcu;
struct fib_nh fib_nh[0];
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index d83071d..cb91f67 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -81,6 +81,7 @@ config IP_MULTIPLE_TABLES
config IP_ROUTE_MULTIPATH
bool "IP: equal cost multipath"
depends on IP_ADVANCED_ROUTER
+ select BITREVERSE
help
Normally, the routing tables specify a single action to be taken in
a deterministic manner for a given packet. If you say Y here
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index 28ec3c1..8c8df80 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -15,6 +15,7 @@
#include <asm/uaccess.h>
#include <linux/bitops.h>
+#include <linux/bitrev.h>
#include <linux/types.h>
#include <linux/kernel.h>
#include <linux/jiffies.h>
@@ -57,7 +58,7 @@ static struct hlist_head fib_info_devhash[DEVINDEX_HASHSIZE];
#ifdef CONFIG_IP_ROUTE_MULTIPATH
-static DEFINE_SPINLOCK(fib_multipath_lock);
+static DEFINE_PER_CPU(u8, fib_mp_rr_counter);
#define for_nexthops(fi) { \
int nhsel; const struct fib_nh *nh; \
@@ -261,7 +262,7 @@ static inline int nh_comp(const struct fib_info *fi, const struct fib_info *ofi)
nh->nh_gw != onh->nh_gw ||
nh->nh_scope != onh->nh_scope ||
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- nh->nh_weight != onh->nh_weight ||
+ nh->nh_mp_weight != onh->nh_mp_weight ||
#endif
#ifdef CONFIG_IP_ROUTE_CLASSID
nh->nh_tclassid != onh->nh_tclassid ||
@@ -449,6 +450,43 @@ static int fib_count_nexthops(struct rtnexthop *rtnh, int remaining)
return remaining > 0 ? 0 : nhs;
}
+static void fib_rebalance(struct fib_info *fi)
+{
+ int factor;
+ int total;
+ int w;
+
+ if (fi->fib_nhs < 2)
+ return;
+
+ total = 0;
+ for_nexthops(fi) {
+ if (!(nh->nh_flags & RTNH_F_DEAD))
+ total += nh->nh_mp_weight;
+ } endfor_nexthops(fi);
+
+ if (likely(total != 0)) {
+ factor = DIV_ROUND_UP(total, 8388608);
+ total /= factor;
+ } else {
+ factor = 1;
+ }
+
+ w = 0;
+ change_nexthops(fi) {
+ int upper_bound;
+
+ if (nexthop_nh->nh_flags & RTNH_F_DEAD) {
+ upper_bound = -1;
+ } else {
+ w += nexthop_nh->nh_mp_weight / factor;
+ upper_bound = DIV_ROUND_CLOSEST(256 * w, total);
+ }
+
+ atomic_set(&nexthop_nh->nh_mp_upper_bound, upper_bound);
+ } endfor_nexthops(fi);
+}
+
static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
int remaining, struct fib_config *cfg)
{
@@ -461,7 +499,7 @@ static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
nexthop_nh->nh_flags =
(cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
nexthop_nh->nh_oif = rtnh->rtnh_ifindex;
- nexthop_nh->nh_weight = rtnh->rtnh_hops + 1;
+ nexthop_nh->nh_mp_weight = rtnh->rtnh_hops + 1;
attrlen = rtnh_attrlen(rtnh);
if (attrlen > 0) {
@@ -884,7 +922,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
fi->fib_net->ipv4.fib_num_tclassid_users++;
#endif
#ifdef CONFIG_IP_ROUTE_MULTIPATH
- nh->nh_weight = 1;
+ nh->nh_mp_weight = 1;
#endif
}
@@ -936,8 +974,15 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
change_nexthops(fi) {
fib_info_update_nh_saddr(net, nexthop_nh);
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+ fi->fib_mp_weight += nexthop_nh->nh_mp_weight;
+#endif
} endfor_nexthops(fi)
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+ fib_rebalance(fi);
+#endif
+
link_it:
ofi = fib_find_info(fi);
if (ofi) {
@@ -1050,7 +1095,7 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
goto nla_put_failure;
rtnh->rtnh_flags = nh->nh_flags & 0xFF;
- rtnh->rtnh_hops = nh->nh_weight - 1;
+ rtnh->rtnh_hops = nh->nh_mp_weight - 1;
rtnh->rtnh_ifindex = nh->nh_oif;
if (nh->nh_gw &&
@@ -1130,12 +1175,6 @@ int fib_sync_down_dev(struct net_device *dev, int force)
else if (nexthop_nh->nh_dev == dev &&
nexthop_nh->nh_scope != scope) {
nexthop_nh->nh_flags |= RTNH_F_DEAD;
-#ifdef CONFIG_IP_ROUTE_MULTIPATH
- spin_lock_bh(&fib_multipath_lock);
- fi->fib_power -= nexthop_nh->nh_power;
- nexthop_nh->nh_power = 0;
- spin_unlock_bh(&fib_multipath_lock);
-#endif
dead++;
}
#ifdef CONFIG_IP_ROUTE_MULTIPATH
@@ -1149,6 +1188,10 @@ int fib_sync_down_dev(struct net_device *dev, int force)
fi->fib_flags |= RTNH_F_DEAD;
ret++;
}
+
+#ifdef CONFIG_IP_ROUTE_MULTIPATH
+ fib_rebalance(fi);
+#endif
}
return ret;
@@ -1254,16 +1297,15 @@ int fib_sync_up(struct net_device *dev)
!__in_dev_get_rtnl(dev))
continue;
alive++;
- spin_lock_bh(&fib_multipath_lock);
- nexthop_nh->nh_power = 0;
nexthop_nh->nh_flags &= ~RTNH_F_DEAD;
- spin_unlock_bh(&fib_multipath_lock);
} endfor_nexthops(fi)
if (alive > 0) {
fi->fib_flags &= ~RTNH_F_DEAD;
ret++;
}
+
+ fib_rebalance(fi);
}
return ret;
@@ -1276,49 +1318,19 @@ int fib_sync_up(struct net_device *dev)
void fib_select_multipath(struct fib_result *res)
{
struct fib_info *fi = res->fi;
- int w;
-
- spin_lock_bh(&fib_multipath_lock);
- if (fi->fib_power <= 0) {
- int power = 0;
- change_nexthops(fi) {
- if (!(nexthop_nh->nh_flags & RTNH_F_DEAD)) {
- power += nexthop_nh->nh_weight;
- nexthop_nh->nh_power = nexthop_nh->nh_weight;
- }
- } endfor_nexthops(fi);
- fi->fib_power = power;
- if (power <= 0) {
- spin_unlock_bh(&fib_multipath_lock);
- /* Race condition: route has just become dead. */
- res->nh_sel = 0;
- return;
- }
- }
+ u8 w;
+ w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
- /* w should be random number [0..fi->fib_power-1],
- * it is pretty bad approximation.
- */
-
- w = jiffies % fi->fib_power;
+ for_nexthops(fi) {
+ if (w >= atomic_read(&nh->nh_mp_upper_bound))
+ continue;
- change_nexthops(fi) {
- if (!(nexthop_nh->nh_flags & RTNH_F_DEAD) &&
- nexthop_nh->nh_power) {
- w -= nexthop_nh->nh_power;
- if (w <= 0) {
- nexthop_nh->nh_power--;
- fi->fib_power--;
- res->nh_sel = nhsel;
- spin_unlock_bh(&fib_multipath_lock);
- return;
- }
- }
+ res->nh_sel = nhsel;
+ return;
} endfor_nexthops(fi);
- /* Race condition: route has just become dead. */
+ /* Race condition: route has just become dead */
res->nh_sel = 0;
- spin_unlock_bh(&fib_multipath_lock);
}
#endif
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 0/3] ipv4: Hash-based multipath routing
From: Peter Nørlund @ 2015-06-17 20:08 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
linux-api-u79uwXL29TY76Z2rM5mHXA
This patch series enhances the IPv4 multipath code, adding support for
hash-based multipath.
The multipath algorithm is a per-route attribute (RTA_MP_ALGO) with some
degree of binary compatibility with the old implementation (2.6.12 - 2.6.22),
but without source level compatibility since attributes have different names:
RT_MP_ALG_L3_HASH:
L3 hash-based distribution. This was IP_MP_ALG_NONE, which with the route
cache behaved somewhat like L3-based distribution. This is now the default.
RT_MP_ALG_PER_PACKET:
Per-packet distribution. Was IP_MP_ALG_RR. Uses round-robin.
RT_MP_ALG_DRR, RT_MP_ALG_RANDOM, RT_MP_ALG_WRANDOM:
Unsupported values, but reserved because they existed in 2.6.12 - 2.6.22.
RT_MP_ALG_L4_HASH:
L4 hash-based distribution. This is new.
The traditional modulo approach was replaced by a threshold-based approach,
described in RFC 2992. This reduces disruption in case of link failures or
route changes.
To better support anycast environments where PMTU usually breaks with
multipath, certain ICMP packets are hashed using the header within the
payload, ensuring that ICMP packets are routed over the same path as the
flow they belong to.
As a side effect, the multipath spinlock was removed and the code got faster.
I measured ip_mkroute_input (excl. __mkroute_input) on a Xeon X3350 (2.66GHz)
with two paths and L3 hashing:
1 thread:
Before: ~199.8 cycles(tsc)
After: ~75.2 cycles(tsc)
4 threads:
Before: ~393.9 cycles(tsc)
After: ~77.8 cycles(tsc)
If this patch is accepted, a follow-up patch to iproute2 will also be
submitted.
Best regards,
Peter Nørlund
Peter Nørlund (3):
ipv4: Lock-less per-packet multipath
ipv4: L3 and L4 hash-based multipath routing
ipv4: ICMP packet inspection for multipath
include/net/ip_fib.h | 10 ++-
include/net/route.h | 5 +
include/uapi/linux/rtnetlink.h | 14 ++++
net/ipv4/Kconfig | 1
net/ipv4/fib_frontend.c | 4 +
net/ipv4/fib_semantics.c | 146 +++++++++++++++++++++++++---------------
net/ipv4/icmp.c | 29 +++++++-
net/ipv4/route.c | 108 +++++++++++++++++++++++++++---
net/ipv4/xfrm4_policy.c | 2 -
9 files changed, 246 insertions(+), 73 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 0/5] selftests/kdbus: small fixes
From: Greg Kroah-Hartman @ 2015-06-17 19:11 UTC (permalink / raw)
To: Shuah Khan
Cc: Sergei Zviagintsev, Daniel Mack, David Herrmann, Djalal Harouni,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <5581A962.3040601-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Wed, Jun 17, 2015 at 11:07:46AM -0600, Shuah Khan wrote:
> On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
> > Set of different fixes over selftests/kdbus.
> >
> > These were first sent as independent patches, and in v2 they just
> > properly threaded.
> >
> > Links to previous patches:
> >
> > http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> > http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> > http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> > http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> > http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> >
> > Sergei Zviagintsev (5):
> > selftests/kdbus: handle cap_get_proc() error properly
> > selftests/kdbus: drop useless assignment
> > selftests/kdbus: remove useless initializations from
> > kdbus_clone_userns_test()
> > selftests/kdbus: drop duplicated code from __kdbus_msg_send()
> > selftests/kdbus: fix error paths in __kdbus_msg_send()
> >
> > tools/testing/selftests/kdbus/kdbus-util.c | 46 ++++++++++++------------
> > tools/testing/selftests/kdbus/test-metadata-ns.c | 9 ++---
> > 2 files changed, 27 insertions(+), 28 deletions(-)
> >
>
> Nice work! Thanks for these fixes.
> Greg! Are you going to apply these into your kdbus tree?
Yes, I will take them.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] selftests: add seccomp suite
From: Kees Cook @ 2015-06-17 18:12 UTC (permalink / raw)
To: Michael Ellerman
Cc: LKML, Daniel Borkmann, Shuah Khan, Andy Lutomirski, Will Drewry,
Andrew Morton, Greg KH, Mauro Carvalho Chehab, David S. Miller,
Arnd Bergmann, Joe Perches, Jingoo Han, Linux API
In-Reply-To: <1434526286.28933.2.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
On Wed, Jun 17, 2015 at 12:31 AM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> On Tue, 2015-06-16 at 10:54 -0700, Kees Cook wrote:
>> This imports the existing seccomp test suite into the kernel's selftests
>> tree. It contains extensive testing of seccomp features and corner cases.
>> There remain additional tests to move into the kernel tree, but they have
>> not yet been ported to all the architectures seccomp supports:
>> https://github.com/redpig/seccomp/tree/master/tests
>>
>> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>> MAINTAINERS | 1 +
>> tools/testing/selftests/Makefile | 1 +
>> tools/testing/selftests/seccomp/.gitignore | 1 +
>> tools/testing/selftests/seccomp/Makefile | 10 +
>> tools/testing/selftests/seccomp/seccomp_bpf.c | 2109 ++++++++++++++++++++++++
>> tools/testing/selftests/seccomp/test_harness.h | 537 ++++++
>
>
> Thanks very much for adding this, it would have been very helpful recently when
> I was trying to get seccomp filter working on powerpc :)
>
> I get one failure in TRACE_syscall.syscall_dropped:
>
> seccomp_bpf.c:1394:TRACE_syscall.syscall_dropped:Expected 1 (1) == syscall(207) (18446744073709551615)
>
>
> So it looks like we're returning -1 instead of 1.
>
> That's probably a bug in our handling of the return value, or maybe an
> inconsistency across the arches. I'll try and find time to dig into it.
Ah-ha! Excellent. Did you add an implementation for change_syscall()
in seccomp_bpf.c? I don't have a powerpc method in there. I would have
expected both TRACE_syscall.syscall_redirected and .syscall_dropped to
fail without that.
If you did, maybe something isn't right with regs.SYSCALL_RET ? That's
where the return value being tested on a skipped syscall is stored.
Thanks for testing!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [PATCH v6 00/21] libnvdimm: non-volatile memory devices
From: Dan Williams @ 2015-06-17 17:28 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-nvdimm@lists.01.org, Boaz Harrosh, Rafael J. Wysocki,
Neil Brown, Dave Chinner, Lv Zheng, H. Peter Anvin,
Stephen Rothwell, Rafael J. Wysocki, Robert Moore, Ingo Molnar,
Linux ACPI, jmoyer, Nicholas Moulin, Matthew Wilcox, Ross Zwisler,
Vishal Verma, Jens Axboe, Borislav Petkov, Thomas Gleixner,
Jens Axboe, Greg KH
In-Reply-To: <20150617112654.GA9246@lst.de>
On Wed, Jun 17, 2015 at 4:26 AM, Christoph Hellwig <hch@lst.de> wrote:
> Still not a big fan of the spec, but I guess this is a mosly reasonable
> implementation now.
>
> So for patches 1-16:
>
> Acked-by: Christoph Hellwig <hch@lst.de>
Thanks Christoph!
> The btt integration still needs a proper patch split and review, and
Ok, I'll split all the patches that are dependent on ->rw_bytes() to
their own series, and break out the introduction of ->rw_bytes() to be
patch1 of that series.
> I still detest the mess with the test moduly by heart.
The only remaining risk I see of merging this is that it requires
someone making changes to either the libnvdimm implementation or the
routines being mocked (ioremap, __request_region, etc...) to think
through the implications to the unit test. That's a maintenance
burden given that mocked interfaces are deliberately working around
base assumptions. At a minimum I need to document what rules the
__wrap_* routines in tools/testing/nvdimm/test/iomap.c are violating
and why.
I'm not convinced that the maintenance burden overshadows the benefit
of having this infrastructure readily available. It is the primary
reason we've been able to iterate the code with confidence at this
velocity and magnitude of feedback.
^ permalink raw reply
* Re: [PATCH v5 17/21] libnvdimm: infrastructure for btt devices
From: Dan Williams @ 2015-06-17 17:09 UTC (permalink / raw)
To: Jeff Moyer
Cc: Christoph Hellwig, Matthew Wilcox, Jens Axboe, Stephen Rothwell,
Andrew Morton, Rafael J. Wysocki, Neil Brown, Greg KH,
linux-nvdimm, linux-kernel@vger.kernel.org, Ingo Molnar,
Linux ACPI, linux-api
In-Reply-To: <x49twu6nugs.fsf@segfault.boston.devel.redhat.com>
On Wed, Jun 17, 2015 at 9:57 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
> Dan Williams <dan.j.williams@intel.com> writes:
>
>> On Wed, Jun 17, 2015 at 9:47 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
>>> Christoph Hellwig <hch@lst.de> writes:
>>>
>>>> On Wed, Jun 10, 2015 at 02:46:16PM -0400, Matthew Wilcox wrote:
>>>>> Don't screw up rw_page. The point of rw_page is to read or write a page
>>>>> cache page. It can sleep, and it indicates success by using the page
>>>>> flags. Don't try and scqueeze rw_bytes into it. If you want rw_bytes
>>>>> to be a queue operation, that's one thing, but don't mess with rw_page.
>>>>
>>>> Oh, I forgot about the page manipulating nature. Yes, we'll need a different
>>>> operation in this case.
>>>
>>> I didn't see this addressed in the new patch set. I'm also concerned
>>> about the layering, but I haven't put enough time into it to really make
>>> a better suggestion. I really dislike the idea of yet another device
>>> stacking model in the kernel and I'm worried the code will go in, and the
>>> sysfs interface will end up as a "user abi" and we won't be able to
>>> change it in the future.
>>>
>>> Dan, have you made any progress on this, or do you have plans to?
>>
>> ? in v6 ->rw_bytes() moved from libnvdimm local hackery to a top-level
>> block device operation. Is that your concern or something else?
>
> Hmm, I guess I was conflating two things. I see now that you did move
> the rw_bytes into the block device operations, that looks good. I'll
> table my concerns over yet another stacking model until I can say
> something intelligent about it.
MD and DM guys can jump in here if I mis-characterize, but I believe
the libnvdimm stacking model:
1/ is warranted because ->rw_bytes() is unique to nvdimm devices and
there are plans for other drivers btt-like drivers to stack on top, a
"struct page" driver is an example
2/ avoids the mistakes of the MD and DM stacking implementations by
having a device-model handle in existence *prior* to attaching a
backing device. MD requires the parent block device to be created
first which causes the implementation to jump through hoops trying to
determine when the MD device has lost its "last opener". DM's model
is mostly opaque to sysfs, it just pops into existence after a magic
sequence of ioctls+netlink.
It also solves the "autodetect" problem of needing to scan every block
device in the system, the scanning is asynchronous and contained to a
given nvdimm bus.
^ permalink raw reply
* Re: [PATCH v2 0/5] selftests/kdbus: small fixes
From: Shuah Khan @ 2015-06-17 17:07 UTC (permalink / raw)
To: Sergei Zviagintsev, Greg Kroah-Hartman, Daniel Mack,
David Herrmann, Djalal Harouni
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434558808-16129-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
> Set of different fixes over selftests/kdbus.
>
> These were first sent as independent patches, and in v2 they just
> properly threaded.
>
> Links to previous patches:
>
> http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei-E844GTqJAzo@public.gmane.org
> http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>
> Sergei Zviagintsev (5):
> selftests/kdbus: handle cap_get_proc() error properly
> selftests/kdbus: drop useless assignment
> selftests/kdbus: remove useless initializations from
> kdbus_clone_userns_test()
> selftests/kdbus: drop duplicated code from __kdbus_msg_send()
> selftests/kdbus: fix error paths in __kdbus_msg_send()
>
> tools/testing/selftests/kdbus/kdbus-util.c | 46 ++++++++++++------------
> tools/testing/selftests/kdbus/test-metadata-ns.c | 9 ++---
> 2 files changed, 27 insertions(+), 28 deletions(-)
>
Nice work! Thanks for these fixes.
Greg! Are you going to apply these into your kdbus tree?
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH v5 17/21] libnvdimm: infrastructure for btt devices
From: Jeff Moyer @ 2015-06-17 16:57 UTC (permalink / raw)
To: Dan Williams
Cc: Christoph Hellwig, Matthew Wilcox, Jens Axboe, Stephen Rothwell,
Andrew Morton, Rafael J. Wysocki, Neil Brown, Greg KH,
linux-nvdimm, linux-kernel@vger.kernel.org, Ingo Molnar,
Linux ACPI, linux-api
In-Reply-To: <CAPcyv4g0CVzhDDiUjc3=5=Bx1HUFjJ7XmCJfxPG1oQAoZ8yvug@mail.gmail.com>
Dan Williams <dan.j.williams@intel.com> writes:
> On Wed, Jun 17, 2015 at 9:47 AM, Jeff Moyer <jmoyer@redhat.com> wrote:
>> Christoph Hellwig <hch@lst.de> writes:
>>
>>> On Wed, Jun 10, 2015 at 02:46:16PM -0400, Matthew Wilcox wrote:
>>>> Don't screw up rw_page. The point of rw_page is to read or write a page
>>>> cache page. It can sleep, and it indicates success by using the page
>>>> flags. Don't try and scqueeze rw_bytes into it. If you want rw_bytes
>>>> to be a queue operation, that's one thing, but don't mess with rw_page.
>>>
>>> Oh, I forgot about the page manipulating nature. Yes, we'll need a different
>>> operation in this case.
>>
>> I didn't see this addressed in the new patch set. I'm also concerned
>> about the layering, but I haven't put enough time into it to really make
>> a better suggestion. I really dislike the idea of yet another device
>> stacking model in the kernel and I'm worried the code will go in, and the
>> sysfs interface will end up as a "user abi" and we won't be able to
>> change it in the future.
>>
>> Dan, have you made any progress on this, or do you have plans to?
>
> ? in v6 ->rw_bytes() moved from libnvdimm local hackery to a top-level
> block device operation. Is that your concern or something else?
Hmm, I guess I was conflating two things. I see now that you did move
the rw_bytes into the block device operations, that looks good. I'll
table my concerns over yet another stacking model until I can say
something intelligent about it.
Cheers,
Jeff
^ permalink raw reply
* Re: [PATCH v5 17/21] libnvdimm: infrastructure for btt devices
From: Dan Williams @ 2015-06-17 16:50 UTC (permalink / raw)
To: Jeff Moyer
Cc: Christoph Hellwig, Matthew Wilcox, Jens Axboe, Stephen Rothwell,
Andrew Morton, Rafael J. Wysocki, Neil Brown, Greg KH,
linux-nvdimm,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ingo Molnar,
Linux ACPI, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <x49381qp9ic.fsf-RRHT56Q3PSP4kTEheFKJxxDDeQx5vsVwAInAS/Ez/D0@public.gmane.org>
On Wed, Jun 17, 2015 at 9:47 AM, Jeff Moyer <jmoyer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> writes:
>
>> On Wed, Jun 10, 2015 at 02:46:16PM -0400, Matthew Wilcox wrote:
>>> Don't screw up rw_page. The point of rw_page is to read or write a page
>>> cache page. It can sleep, and it indicates success by using the page
>>> flags. Don't try and scqueeze rw_bytes into it. If you want rw_bytes
>>> to be a queue operation, that's one thing, but don't mess with rw_page.
>>
>> Oh, I forgot about the page manipulating nature. Yes, we'll need a different
>> operation in this case.
>
> I didn't see this addressed in the new patch set. I'm also concerned
> about the layering, but I haven't put enough time into it to really make
> a better suggestion. I really dislike the idea of yet another device
> stacking model in the kernel and I'm worried the code will go in, and the
> sysfs interface will end up as a "user abi" and we won't be able to
> change it in the future.
>
> Dan, have you made any progress on this, or do you have plans to?
? in v6 ->rw_bytes() moved from libnvdimm local hackery to a top-level
block device operation. Is that your concern or something else?
^ permalink raw reply
* Re: [PATCH v5 17/21] libnvdimm: infrastructure for btt devices
From: Jeff Moyer @ 2015-06-17 16:47 UTC (permalink / raw)
To: Christoph Hellwig, Dan Williams
Cc: Matthew Wilcox, axboe-tSWWG44O7X1aa/9Udqfwiw,
sfr-3FnU+UHB4dNDw9hX6IcOSA, akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
rafael-DgEjT+Ai2ygdnm+yROfE0A, neilb-l3A5Bk7waGM,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-nvdimm-y27Ovi1pjclAfugRpC6u6w,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, mingo-DgEjT+Ai2ygdnm+yROfE0A,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150611072812.GB1905-jcswGhMUV9g@public.gmane.org>
Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org> writes:
> On Wed, Jun 10, 2015 at 02:46:16PM -0400, Matthew Wilcox wrote:
>> Don't screw up rw_page. The point of rw_page is to read or write a page
>> cache page. It can sleep, and it indicates success by using the page
>> flags. Don't try and scqueeze rw_bytes into it. If you want rw_bytes
>> to be a queue operation, that's one thing, but don't mess with rw_page.
>
> Oh, I forgot about the page manipulating nature. Yes, we'll need a different
> operation in this case.
I didn't see this addressed in the new patch set. I'm also concerned
about the layering, but I haven't put enough time into it to really make
a better suggestion. I really dislike the idea of yet another device
stacking model in the kernel and I'm worried the code will go in, and the
sysfs interface will end up as a "user abi" and we won't be able to
change it in the future.
Dan, have you made any progress on this, or do you have plans to?
Cheers,
Jeff
^ permalink raw reply
* Re: [PATCH] selftests/kdbus: handle cap_get_proc() error properly
From: Sergei Zviagintsev @ 2015-06-17 16:46 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150617143720.GA19114-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Hi Greg,
On Wed, Jun 17, 2015 at 07:37:20AM -0700, Greg Kroah-Hartman wrote:
> You sent 5 patches, yet no hints as to what order they should be applied
> in :(
>
> Please resend in a patch series that is properly threaded so we can
> handle this correctly.
Thank you for feedback! I've resent series threaded:
http://lkml.kernel.org/g/1434558808-16129-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org
It was unclear for me whether to use threaded layout and whether not,
but now it seems that I got it. Though those applied fine in any order
and had no chances to piss of a kernel maintainer :)
^ permalink raw reply
* [PATCH v2 5/5] selftests/kdbus: fix error paths in __kdbus_msg_send()
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel, linux-api, Sergei Zviagintsev
In-Reply-To: <1434558808-16129-1-git-send-email-sergei@s15v.net>
Handle errors properly, free allocated resources.
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
tools/testing/selftests/kdbus/kdbus-util.c | 31 ++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index ae81da63d810..29a0cb1aace2 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -452,8 +452,8 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
uint64_t cmd_flags,
int cancel_fd)
{
- struct kdbus_cmd_send *cmd;
- struct kdbus_msg *msg;
+ struct kdbus_cmd_send *cmd = NULL;
+ struct kdbus_msg *msg = NULL;
const char ref1[1024 * 128 + 3] = "0123456789_0";
const char ref2[] = "0123456789_1";
struct kdbus_item *item;
@@ -476,14 +476,14 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (write(memfd, "kdbus memfd 1234567", 19) != 19) {
ret = -errno;
kdbus_printf("writing to memfd failed: %m\n");
- return ret;
+ goto out;
}
ret = sys_memfd_seal_set(memfd);
if (ret < 0) {
ret = -errno;
kdbus_printf("memfd sealing failed: %m\n");
- return ret;
+ goto out;
}
size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_memfd));
@@ -496,7 +496,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (!msg) {
ret = -errno;
kdbus_printf("unable to malloc()!?\n");
- return ret;
+ goto out;
}
if (dst_id == KDBUS_DST_ID_BROADCAST)
@@ -514,7 +514,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (timeout) {
ret = clock_gettime(CLOCK_MONOTONIC_COARSE, &now);
if (ret < 0)
- return ret;
+ goto out;
msg->timeout_ns = now.tv_sec * 1000000000ULL +
now.tv_nsec + timeout;
@@ -565,6 +565,12 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
size += KDBUS_ITEM_SIZE(sizeof(cancel_fd));
cmd = malloc(size);
+ if (!cmd) {
+ ret = -errno;
+ kdbus_printf("unable to malloc()!?\n");
+ goto out;
+ }
+
cmd->size = size;
cmd->flags = cmd_flags;
cmd->msg_address = (uintptr_t)msg;
@@ -579,12 +585,9 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
}
ret = kdbus_cmd_send(conn->fd, cmd);
- if (memfd >= 0)
- close(memfd);
-
if (ret < 0) {
kdbus_printf("error sending message: %d (%m)\n", ret);
- return ret;
+ goto out;
}
if (cmd_flags & KDBUS_SEND_SYNC_REPLY) {
@@ -598,13 +601,17 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
ret = kdbus_free(conn, cmd->reply.offset);
if (ret < 0)
- return ret;
+ goto out;
}
+out:
free(msg);
free(cmd);
- return 0;
+ if (memfd >= 0)
+ close(memfd);
+
+ return ret < 0 ? ret : 0;
}
int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 4/5] selftests/kdbus: drop duplicated code from __kdbus_msg_send()
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel, linux-api, Sergei Zviagintsev
In-Reply-To: <1434558808-16129-1-git-send-email-sergei@s15v.net>
Set value of `size' in one step instead of four.
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
tools/testing/selftests/kdbus/kdbus-util.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index f9102264cfb9..ae81da63d810 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -462,10 +462,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
int memfd = -1;
int ret;
- size = sizeof(*msg);
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
+ size = sizeof(*msg) + 3 * KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
if (dst_id == KDBUS_DST_ID_BROADCAST)
size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_bloom_filter)) + 64;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 3/5] selftests/kdbus: remove useless initializations from kdbus_clone_userns_test()
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Sergei Zviagintsev
In-Reply-To: <1434558808-16129-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
Do not initialize efd, unpriv_conn_id, userns_conn_id and monitor. These
vars are assigned to values later in code while initial values are never
used.
Signed-off-by: Sergei Zviagintsev <sergei-E844GTqJAzo@public.gmane.org>
---
tools/testing/selftests/kdbus/test-metadata-ns.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/kdbus/test-metadata-ns.c b/tools/testing/selftests/kdbus/test-metadata-ns.c
index 2cb1d4d2a5be..ccdfae06922b 100644
--- a/tools/testing/selftests/kdbus/test-metadata-ns.c
+++ b/tools/testing/selftests/kdbus/test-metadata-ns.c
@@ -281,16 +281,13 @@ out:
static int kdbus_clone_userns_test(const char *bus,
struct kdbus_conn *conn)
{
- int ret;
- int status;
- int efd = -1;
+ int ret, status, efd;
pid_t pid, ppid;
- uint64_t unpriv_conn_id = 0;
- uint64_t userns_conn_id = 0;
+ uint64_t unpriv_conn_id, userns_conn_id;
struct kdbus_msg *msg;
const struct kdbus_item *item;
struct kdbus_pids expected_pids;
- struct kdbus_conn *monitor = NULL;
+ struct kdbus_conn *monitor;
kdbus_printf("STARTING TEST 'metadata-ns'.\n");
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 2/5] selftests/kdbus: drop useless assignment
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Sergei Zviagintsev
In-Reply-To: <1434558808-16129-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
Assign returned file descriptor directly to `fd', without intermediate
`ret' variable.
Signed-off-by: Sergei Zviagintsev <sergei-E844GTqJAzo@public.gmane.org>
---
tools/testing/selftests/kdbus/kdbus-util.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index a3e49b04f3a7..f9102264cfb9 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -408,11 +408,9 @@ int sys_memfd_create(const char *name, __u64 size)
{
int ret, fd;
- ret = syscall(__NR_memfd_create, name, MFD_ALLOW_SEALING);
- if (ret < 0)
- return ret;
-
- fd = ret;
+ fd = syscall(__NR_memfd_create, name, MFD_ALLOW_SEALING);
+ if (fd < 0)
+ return fd;
ret = ftruncate(fd, size);
if (ret < 0) {
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 1/5] selftests/kdbus: handle cap_get_proc() error properly
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel, linux-api, Sergei Zviagintsev
In-Reply-To: <1434558808-16129-1-git-send-email-sergei@s15v.net>
Fix typo in checking error value of cap_get_proc(): cap -> caps
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
tools/testing/selftests/kdbus/kdbus-util.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index 1e267d585a14..a3e49b04f3a7 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -1547,7 +1547,7 @@ int test_is_capable(int cap, ...)
cap_t caps;
caps = cap_get_proc();
- if (!cap) {
+ if (!caps) {
ret = -errno;
kdbus_printf("error cap_get_proc(): %d (%m)\n", ret);
return ret;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/5] selftests/kdbus: small fixes
From: Sergei Zviagintsev @ 2015-06-17 16:33 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Sergei Zviagintsev
Set of different fixes over selftests/kdbus.
These were first sent as independent patches, and in v2 they just
properly threaded.
Links to previous patches:
http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org
http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei-E844GTqJAzo@public.gmane.org
http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei-E844GTqJAzo@public.gmane.org
http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei-E844GTqJAzo@public.gmane.org
http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei-E844GTqJAzo@public.gmane.org
Sergei Zviagintsev (5):
selftests/kdbus: handle cap_get_proc() error properly
selftests/kdbus: drop useless assignment
selftests/kdbus: remove useless initializations from
kdbus_clone_userns_test()
selftests/kdbus: drop duplicated code from __kdbus_msg_send()
selftests/kdbus: fix error paths in __kdbus_msg_send()
tools/testing/selftests/kdbus/kdbus-util.c | 46 ++++++++++++------------
tools/testing/selftests/kdbus/test-metadata-ns.c | 9 ++---
2 files changed, 27 insertions(+), 28 deletions(-)
--
1.8.3.1
^ permalink raw reply
* Re: [PATCH] selftests/kdbus: handle cap_get_proc() error properly
From: Greg Kroah-Hartman @ 2015-06-17 14:37 UTC (permalink / raw)
To: Sergei Zviagintsev
Cc: Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434550697-24785-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
On Wed, Jun 17, 2015 at 05:18:13PM +0300, Sergei Zviagintsev wrote:
> Fix typo in checking error value of cap_get_proc(): cap -> caps
>
> Signed-off-by: Sergei Zviagintsev <sergei-E844GTqJAzo@public.gmane.org>
> ---
> tools/testing/selftests/kdbus/kdbus-util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
You sent 5 patches, yet no hints as to what order they should be applied
in :(
Please resend in a patch series that is properly threaded so we can
handle this correctly.
thanks,
greg k-h
^ permalink raw reply
* [PATCH] selftests/kdbus: fix error paths in __kdbus_msg_send()
From: Sergei Zviagintsev @ 2015-06-17 14:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel, linux-api, Sergei Zviagintsev
Handle errors properly, free allocated resources.
Signed-off-by: Sergei Zviagintsev <sergei@s15v.net>
---
tools/testing/selftests/kdbus/kdbus-util.c | 31 ++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index ae81da63d810..29a0cb1aace2 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -452,8 +452,8 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
uint64_t cmd_flags,
int cancel_fd)
{
- struct kdbus_cmd_send *cmd;
- struct kdbus_msg *msg;
+ struct kdbus_cmd_send *cmd = NULL;
+ struct kdbus_msg *msg = NULL;
const char ref1[1024 * 128 + 3] = "0123456789_0";
const char ref2[] = "0123456789_1";
struct kdbus_item *item;
@@ -476,14 +476,14 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (write(memfd, "kdbus memfd 1234567", 19) != 19) {
ret = -errno;
kdbus_printf("writing to memfd failed: %m\n");
- return ret;
+ goto out;
}
ret = sys_memfd_seal_set(memfd);
if (ret < 0) {
ret = -errno;
kdbus_printf("memfd sealing failed: %m\n");
- return ret;
+ goto out;
}
size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_memfd));
@@ -496,7 +496,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (!msg) {
ret = -errno;
kdbus_printf("unable to malloc()!?\n");
- return ret;
+ goto out;
}
if (dst_id == KDBUS_DST_ID_BROADCAST)
@@ -514,7 +514,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
if (timeout) {
ret = clock_gettime(CLOCK_MONOTONIC_COARSE, &now);
if (ret < 0)
- return ret;
+ goto out;
msg->timeout_ns = now.tv_sec * 1000000000ULL +
now.tv_nsec + timeout;
@@ -565,6 +565,12 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
size += KDBUS_ITEM_SIZE(sizeof(cancel_fd));
cmd = malloc(size);
+ if (!cmd) {
+ ret = -errno;
+ kdbus_printf("unable to malloc()!?\n");
+ goto out;
+ }
+
cmd->size = size;
cmd->flags = cmd_flags;
cmd->msg_address = (uintptr_t)msg;
@@ -579,12 +585,9 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
}
ret = kdbus_cmd_send(conn->fd, cmd);
- if (memfd >= 0)
- close(memfd);
-
if (ret < 0) {
kdbus_printf("error sending message: %d (%m)\n", ret);
- return ret;
+ goto out;
}
if (cmd_flags & KDBUS_SEND_SYNC_REPLY) {
@@ -598,13 +601,17 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
ret = kdbus_free(conn, cmd->reply.offset);
if (ret < 0)
- return ret;
+ goto out;
}
+out:
free(msg);
free(cmd);
- return 0;
+ if (memfd >= 0)
+ close(memfd);
+
+ return ret < 0 ? ret : 0;
}
int kdbus_msg_send(const struct kdbus_conn *conn, const char *name,
--
1.8.3.1
^ permalink raw reply related
* [PATCH] selftests/kdbus: drop duplicated code from __kdbus_msg_send()
From: Sergei Zviagintsev @ 2015-06-17 14:18 UTC (permalink / raw)
To: Greg Kroah-Hartman, Daniel Mack, David Herrmann, Djalal Harouni
Cc: Shuah Khan, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, Sergei Zviagintsev
Set value of `size' in one step instead of four.
Signed-off-by: Sergei Zviagintsev <sergei-E844GTqJAzo@public.gmane.org>
---
tools/testing/selftests/kdbus/kdbus-util.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/tools/testing/selftests/kdbus/kdbus-util.c b/tools/testing/selftests/kdbus/kdbus-util.c
index f9102264cfb9..ae81da63d810 100644
--- a/tools/testing/selftests/kdbus/kdbus-util.c
+++ b/tools/testing/selftests/kdbus/kdbus-util.c
@@ -462,10 +462,7 @@ static int __kdbus_msg_send(const struct kdbus_conn *conn,
int memfd = -1;
int ret;
- size = sizeof(*msg);
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
- size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
+ size = sizeof(*msg) + 3 * KDBUS_ITEM_SIZE(sizeof(struct kdbus_vec));
if (dst_id == KDBUS_DST_ID_BROADCAST)
size += KDBUS_ITEM_SIZE(sizeof(struct kdbus_bloom_filter)) + 64;
--
1.8.3.1
^ 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