Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next v5 2/2] net: thunderx: add timestamping support
From: Aleksey Makarov @ 2018-01-08 17:12 UTC (permalink / raw)
  To: Richard Cochran
  Cc: netdev, linux-arm-kernel, linux-kernel, Goutham, Sunil,
	Radoslaw Biernacki, Robert Richter, David Daney,
	Philippe Ombredanne, Sunil Goutham
In-Reply-To: <20171211233220.d72rys7nci4lqqd5@localhost>



On 12.12.2017 05:32, Richard Cochran wrote:
> On Mon, Dec 11, 2017 at 05:14:31PM +0300, Aleksey Makarov wrote:
>> diff --git a/drivers/net/ethernet/cavium/thunder/nic.h b/drivers/net/ethernet/cavium/thunder/nic.h
>> index 4a02e618e318..204b234beb9d 100644
>> --- a/drivers/net/ethernet/cavium/thunder/nic.h
>> +++ b/drivers/net/ethernet/cavium/thunder/nic.h
>> @@ -263,6 +263,8 @@ struct nicvf_drv_stats {
>>  	struct u64_stats_sync   syncp;
>>  };
>>  
>> +struct cavium_ptp;
>> +
>>  struct nicvf {
>>  	struct nicvf		*pnicvf;
>>  	struct net_device	*netdev;
>> @@ -312,6 +314,12 @@ struct nicvf {
>>  	struct tasklet_struct	qs_err_task;
>>  	struct work_struct	reset_task;
>>  
>> +	/* PTP timestamp */
>> +	struct cavium_ptp	*ptp_clock;
>> +	bool			hw_rx_tstamp;
>> +	struct sk_buff		*ptp_skb;
>> +	atomic_t		tx_ptp_skbs;
> 
> It is disturbing that the above two fields are set in different
> places.  Shouldn't they be unified into one logical lock?

No, they should not as they are not quite related.

`tx_ptp_skbs` is set when the hardware is sending a packet that requires
timestamping.  Cavium hardware can not process more than one
such packet at once so this is set each time the driver submits
a packet that requires timestamping to the send queue and clears
each time it receives the entry on the completion queue saying
that such packet was sent.

So `tx_ptp_skbs` prevents driver from submitting more than one
packet that requires timestamping to the hardware for transmitting.

When that packet is sent, hardware inserts two entries to
the completion queue.  First is the regular CQE_TYPE_SEND entry
that signals that the packet was sent.  The second is CQE_TYPE_SEND_PTP
that contains the actual timestamp for that packet.

`ptp_skb` is initialized in the handler for the CQE_TYPE_SEND
entry and is used and zeroed in the handler for the CQE_TYPE_SEND_PTP
entry.

So `ptp_skb` is used to hold the pointer to the packet between
the calls to CQE_TYPE_SEND and CQE_TYPE_SEND_PTP handlers.

I am going to add those comments to the sources, fix other issues and
send v6 in short time.

Thank you
Aleksey Makarov

> Here you clear them together:
> 
>> +static void nicvf_snd_ptp_handler(struct net_device *netdev,
>> +				  struct cqe_send_t *cqe_tx)
>> +{
>> +	struct nicvf *nic = netdev_priv(netdev);
>> +	struct skb_shared_hwtstamps ts;
>> +	u64 ns;
>> +
>> +	nic = nic->pnicvf;
>> +
>> +	/* Sync for 'ptp_skb' */
>> +	smp_rmb();
>> +
>> +	/* New timestamp request can be queued now */
>> +	atomic_set(&nic->tx_ptp_skbs, 0);
>> +
>> +	/* Check for timestamp requested skb */
>> +	if (!nic->ptp_skb)
>> +		return;
>> +
>> +	/* Check if timestamping is timedout, which is set to 10us */
>> +	if (cqe_tx->send_status == CQ_TX_ERROP_TSTMP_TIMEOUT ||
>> +	    cqe_tx->send_status == CQ_TX_ERROP_TSTMP_CONFLICT)
>> +		goto no_tstamp;
>> +
>> +	/* Get the timestamp */
>> +	memset(&ts, 0, sizeof(ts));
>> +	ns = cavium_ptp_tstamp2time(nic->ptp_clock, cqe_tx->ptp_timestamp);
>> +	ts.hwtstamp = ns_to_ktime(ns);
>> +	skb_tstamp_tx(nic->ptp_skb, &ts);
>> +
>> +no_tstamp:
>> +	/* Free the original skb */
>> +	dev_kfree_skb_any(nic->ptp_skb);
>> +	nic->ptp_skb = NULL;
>> +	/* Sync 'ptp_skb' */
>> +	smp_wmb();
>> +}
>> +
> 
> but here you set the one:
> 
>> @@ -657,7 +697,12 @@ static void nicvf_snd_pkt_handler(struct net_device *netdev,
>>  		prefetch(skb);
>>  		(*tx_pkts)++;
>>  		*tx_bytes += skb->len;
>> -		napi_consume_skb(skb, budget);
>> +		/* If timestamp is requested for this skb, don't free it */
>> +		if (skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS &&
>> +		    !nic->pnicvf->ptp_skb)
>> +			nic->pnicvf->ptp_skb = skb;
>> +		else
>> +			napi_consume_skb(skb, budget);
>>  		sq->skbuff[cqe_tx->sqe_ptr] = (u64)NULL;
>>  	} else {
>>  		/* In case of SW TSO on 88xx, only last segment will have
> 
> here you clear one:
> 
>> @@ -1319,12 +1382,28 @@ int nicvf_stop(struct net_device *netdev)
>>  
>>  	nicvf_free_cq_poll(nic);
>>  
>> +	/* Free any pending SKB saved to receive timestamp */
>> +	if (nic->ptp_skb) {
>> +		dev_kfree_skb_any(nic->ptp_skb);
>> +		nic->ptp_skb = NULL;
>> +	}
>> +
>>  	/* Clear multiqset info */
>>  	nic->pnicvf = nic;
>>  
>>  	return 0;
>>  }
> 
> here you clear both:
> 
>> @@ -1394,6 +1473,12 @@ int nicvf_open(struct net_device *netdev)
>>  	if (nic->sqs_mode)
>>  		nicvf_get_primary_vf_struct(nic);
>>  
>> +	/* Configure PTP timestamp */
>> +	if (nic->ptp_clock)
>> +		nicvf_config_hw_rx_tstamp(nic, nic->hw_rx_tstamp);
>> +	atomic_set(&nic->tx_ptp_skbs, 0);
>> +	nic->ptp_skb = NULL;
>> +
>>  	/* Configure receive side scaling and MTU */
>>  	if (!nic->sqs_mode) {
>>  		nicvf_rss_init(nic);
> 
> here you set the other:
> 
>> @@ -1385,6 +1388,29 @@ nicvf_sq_add_hdr_subdesc(struct nicvf *nic, struct snd_queue *sq, int qentry,
>>  		hdr->inner_l3_offset = skb_network_offset(skb) - 2;
>>  		this_cpu_inc(nic->pnicvf->drv_stats->tx_tso);
>>  	}
>> +
>> +	/* Check if timestamp is requested */
>> +	if (!(skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP)) {
>> +		skb_tx_timestamp(skb);
>> +		return;
>> +	}
>> +
>> +	/* Tx timestamping not supported along with TSO, so ignore request */
>> +	if (skb_shinfo(skb)->gso_size)
>> +		return;
>> +
>> +	/* HW supports only a single outstanding packet to timestamp */
>> +	if (!atomic_add_unless(&nic->pnicvf->tx_ptp_skbs, 1, 1))
>> +		return;
>> +
>> +	/* Mark the SKB for later reference */
>> +	skb_shinfo(skb)->tx_flags |= SKBTX_IN_PROGRESS;
>> +
>> +	/* Finally enable timestamp generation
>> +	 * Since 'post_cqe' is also set, two CQEs will be posted
>> +	 * for this packet i.e CQE_TYPE_SEND and CQE_TYPE_SEND_PTP.
>> +	 */
>> +	hdr->tstmp = 1;
>>  }
> 
> and so it is completely non-obvious whether this is race free or not.
> 
> Thanks,
> Richard
> 

^ permalink raw reply

* Re: [PATCH 02/18] Documentation: document nospec helpers
From: Mark Rutland @ 2018-01-08 17:09 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Dan Williams, linux-kernel, linux-arch, peterz, netdev,
	Will Deacon, gregkh, tglx, torvalds, alan
In-Reply-To: <20180108092917.591359aa@lwn.net>

Hi Jon,

On Mon, Jan 08, 2018 at 09:29:17AM -0700, Jonathan Corbet wrote:
> On Fri, 05 Jan 2018 17:10:03 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Document the rationale and usage of the new nospec*() helpers.
> 
> I have just a couple of overall comments.
> 
>  - It would be nice if the document were done in RST and placed in the
>    core-API manual, perhaps using kerneldoc comments for the macros
>    themselves.  It's already 99.9% RST now, so the changes required would
>    be minimal.

Is there any quickstart guide to RST that you can recommend?

I'm happy to clean up the documentation; I'm just not familiar with RST.

>  - More importantly: is there any way at all to give guidance to
>    developers wondering *when* they should use these primitives?  I think
>    it would be easy to create a situation where they don't get used where
>    they are really needed; meanwhile, there may well be a flood of
>    "helpful" patches adding them where they make no sense at all.

This is on my TODO list.

The unfortunate truth is that it's likely to be a subjective judgement
call in many cases, depending on how likely it is that the user can
influence the code in question, so it's difficult to provide
hard-and-fast rules.

Thanks,
Mark.

^ permalink raw reply

* Re: [PATCH iproute2 v3 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:07 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>


[-- Attachment #1.1: Type: text/plain, Size: 944 bytes --]


Please ignore series: wrong series number (3).
Sorry for noise.


> With this series I want to improve printing of tclass,
> flowlabel and encapsulation limit (encap_limit) options.
> 
> Everything within this series is open for your comments,
> suggestions and criticism.
> 
> See individual patch description message for details.
> 
> Thanks,
> Serhii
> 
> Serhey Popovych (7):
>   ip6/tunnel: Fix tclass output
>   ip6tnl/tunnel: Do not print obscure flowinfo
>   ip6/tunnel: Unify tclass printing
>   ip6/tunnel: Unify flowlabel printing
>   ip6/tunnel: Unify encap_limit printing
>   gre6/tunnel: Output flowlabel after tclass
>   ip6tnl/tunnel: Output hoplimit before encapsulation limit
> 
>  ip/link_gre6.c   |   54 ++++++++++++++++------------------------------
>  ip/link_ip6tnl.c |   63 ++++++++++++++++++------------------------------------
>  2 files changed, 40 insertions(+), 77 deletions(-)
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]

^ permalink raw reply

* [PATCH iproute2 7/7] ip6tnl/tunnel: Output hoplimit before encapsulation limit
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

To follow gre6 output print hoplimit before encapsulation
limit in link_ip6tnl.c.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 379eb33..bbc7878 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,6 +387,13 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
 	}
 
+	if (tb[IFLA_IPTUN_TTL]) {
+		print_uint(PRINT_ANY,
+			   "ttl",
+			   "hoplimit %u ",
+			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
+	}
+
 	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
@@ -398,12 +405,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
-	if (tb[IFLA_IPTUN_TTL])
-		print_uint(PRINT_ANY,
-			   "ttl",
-			   "hoplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
-
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 6/7] gre6/tunnel: Output flowlabel after tclass
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

To follow ip6tnl output print flowlabel after tclass
in link_gre6.c.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 8014207..55bd1fb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -444,18 +444,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
-	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-		print_bool(PRINT_ANY,
-			   "ip6_tnl_f_use_orig_flowlabel",
-			   "flowlabel inherit ",
-			   true);
-	} else if (tb[IFLA_GRE_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
-
-		snprintf(s2, sizeof(s2), "0x%05x", val);
-		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
-	}
-
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
@@ -468,6 +456,18 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
+	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+		print_bool(PRINT_ANY,
+			   "ip6_tnl_f_use_orig_flowlabel",
+			   "flowlabel inherit ",
+			   true);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
+
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
+	}
+
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 5/7] ip6/tunnel: Unify encap_limit printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

Use %u format specifier to print it in link_gre6.c and
make code more readable.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   11 ++++-------
 ip/link_ip6tnl.c |   12 ++++++------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 1205946..8014207 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -433,18 +433,15 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			print_int(PRINT_JSON, "ttl", NULL, ttl);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
-		int encap_limit = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
+	} else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
 
-		print_int(PRINT_ANY,
-			  "encap_limit",
-			  "encaplimit %d ",
-			  encap_limit);
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 7000056..379eb33 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,16 +387,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_IPTUN_ENCAP_LIMIT])
-		print_uint(PRINT_ANY,
-			   "encap_limit",
-			   "encaplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]));
+	} else if (tb[IFLA_IPTUN_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]);
+
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
+	}
 
 	if (tb[IFLA_IPTUN_TTL])
 		print_uint(PRINT_ANY,
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 4/7] ip6/tunnel: Unify flowlabel printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

Use @s2 buffer to store string representation of
flowlabel and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

Omit flowlabel from output if no flowinfo attribute
is given and IP6_TNL_F_USE_ORIG_FLOWLABEL isn't set.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   15 ++++-----------
 ip/link_ip6tnl.c |   14 ++++----------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a02dd4a..1205946 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -452,18 +452,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-		} else {
-			fprintf(f, "flowlabel 0x%05x ",
-				ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 05322fd..7000056 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -421,17 +421,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
-		} else {
-			printf("flowlabel 0x%05x ",
-			       ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 3/7] ip6/tunnel: Unify tclass printing
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

Use @s2 buffer to store string representation of
tclass and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

While there use __u32 for flowinfo in link_gre6.c
and check for IFLA_GRE_FLOWINFO attribute presense.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   16 +++++-----------
 ip/link_ip6tnl.c |   16 +++++-----------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 87c313c..a02dd4a 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -380,7 +380,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
-	unsigned int flowinfo = 0;
+	__u32 flowinfo = 0;
 	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
 
 	if (!tb)
@@ -471,17 +471,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-			snprintf(b1, sizeof(b1), "0x%02x",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			fprintf(f, "tclass 0x%02x ",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index e084975..05322fd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -404,22 +404,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "hoplimit %u ",
 			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
 
-	if (flags & IP6_TNL_F_USE_ORIG_TCLASS)
+	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	else if (tb[IFLA_IPTUN_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			printf("tclass 0x%02x ", (__u8)(val >> 20));
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 2/7] ip6tnl/tunnel: Do not print obscure flowinfo
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

It is implementation internal and main purpose
of printing it seems debugging.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8e84ed0..e084975 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -440,16 +440,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (is_json_context()) {
-		SPRINT_BUF(flwinfo);
-
-		snprintf(flwinfo, sizeof(flwinfo), "0x%08x", ntohl(flowinfo));
-		print_string(PRINT_JSON, "flowinfo", NULL, flwinfo);
-	} else {
-		printf("(flowinfo 0x%08x) ", ntohl(flowinfo));
-
-	}
-
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 1/7] ip6/tunnel: Fix tclass output
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431198-29362-1-git-send-email-serhe.popovych@gmail.com>

In link_gre6.c it seems copy paste error: tclass is 8 bits,
not 20 as flowlabel.

In link_iptnl.c rename "flowinfo_tclass" to "tclass" as it
correct name since flowinfo is implementation internal name
used to label combined within u32 attribute tclass and
flowlabel.

Fixes: 1facc1c61c07 ("ip: link_ip6tnl.c: add json output support")
Fixes: 2e706e12d9b0 ("Merge branch 'master' into net-next")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |    2 +-
 ip/link_ip6tnl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..87c313c 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -475,7 +475,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (is_json_context()) {
 			SPRINT_BUF(b1);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
+			snprintf(b1, sizeof(b1), "0x%02x",
 				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
 			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 84205b1..8e84ed0 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -416,7 +416,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			SPRINT_BUF(b1);
 
 			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "flowinfo_tclass", NULL, b1);
+			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
 			printf("tclass 0x%02x ", (__u8)(val >> 20));
 		}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:06 UTC (permalink / raw)
  To: netdev

With this series I want to improve printing of tclass,
flowlabel and encapsulation limit (encap_limit) options.

Everything within this series is open for your comments,
suggestions and criticism.

See individual patch description message for details.

Thanks,
Serhii

Serhey Popovych (7):
  ip6/tunnel: Fix tclass output
  ip6tnl/tunnel: Do not print obscure flowinfo
  ip6/tunnel: Unify tclass printing
  ip6/tunnel: Unify flowlabel printing
  ip6/tunnel: Unify encap_limit printing
  gre6/tunnel: Output flowlabel after tclass
  ip6tnl/tunnel: Output hoplimit before encapsulation limit

 ip/link_gre6.c   |   54 ++++++++++++++++------------------------------
 ip/link_ip6tnl.c |   63 ++++++++++++++++++------------------------------------
 2 files changed, 40 insertions(+), 77 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Kalle Valo @ 2018-01-08 17:06 UTC (permalink / raw)
  To: Larry Finger
  Cc: kstewart-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	johannes.berg-ral2JQCrhuEAvxtiuMwx3w, tiwai-l3A5Bk7waGM,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	andrew.zaborowski-ral2JQCrhuEAvxtiuMwx3w, Jia-Ju Bai,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	netdev-u79uwXL29TY76Z2rM5mHXA, colin.king-Z7WLFzj8eWMS+FvcfC7Uqw
In-Reply-To: <fc3e2558-c31f-289d-8413-33fef4f5ca64-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org>

Larry Finger <Larry.Finger-tQ5ms3gMjBLk1uMJSBkQmQ@public.gmane.org> writes:

> On 01/08/2018 10:21 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>
>>> b43_radio_2057_init_post is not called in an interrupt handler
>>> nor holding a spinlock.
>>> The function mdelay in it can be replaced with msleep, to reduce busy wait.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>
>> You submitted an identical patch a week earlier:
>>
>> https://patchwork.kernel.org/patch/10137671/
>>
>> How is this different? Also always add version number to the patch so that the
>> maintainers can follow the changes easily:
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing
>
> I had negative comments on one of those due to the possibility of
> msleep(2) extending as long as 20 msec. Until the author, or someone
> else, can test that this is OK, then the mdelay(2) can only be
> replaced with usleep_range(2000, 3000).
>
> NACK for both.

Ok, patches dropped.

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH bpf] bpf: prevent out-of-bounds speculation
From: Mark Rutland @ 2018-01-08 17:05 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: David S . Miller, Daniel Borkmann, Jann Horn, Linus Torvalds,
	Dan Williams, Peter Zijlstra, Elena Reshetova, Alan Cox, netdev,
	kernel-team, will.deacon
In-Reply-To: <20180105042811.1590965-1-ast@fb.com>

Hi Alexei,

On Thu, Jan 04, 2018 at 08:28:11PM -0800, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> Under speculation, CPUs may mis-predict branches in bounds checks. Thus,
> memory accesses under a bounds check may be speculated even if the
> bounds check fails, providing a primitive for building a side channel.
> 
> To avoid leaking kernel data round up array-based maps and mask the index
> after bounds check, so speculated load with out of bounds index will load
> either valid value from the array or zero from the padded area.

Thanks for putting this together, this certainly looks neat.

I'm a little worried that in the presence of some CPU/compiler
optimisations, the masking may effectively be skipped under speculation.
So I'm not sure how robust this is going to be.

More on that below.

> To avoid duplicating map_lookup functions for root/unpriv always generate
> a sequence of bpf instructions equivalent to map_lookup function for
> array and array_of_maps map types when map was created by unpriv user.
> And unconditionally mask index for percpu_array, since it's fast enough,
> even when max_entries are not rounded to power of 2 for root user,
> since percpu_array doesn't have map_gen_lookup callback yet.

Is there a noticeable slowdown from the masking? Can't we always have
that in place?

> @@ -157,7 +175,7 @@ static void *percpu_array_map_lookup_elem(struct bpf_map *map, void *key)
>  	if (unlikely(index >= array->map.max_entries))
>  		return NULL;
>  
> -	return this_cpu_ptr(array->pptrs[index]);
> +	return this_cpu_ptr(array->pptrs[index & array->index_mask]);

As above, I think this isn't necessarily robust, as CPU/compiler
optimisations can break the dependency on the index_mask, allowing
speculation without a mask.

e.g. a compiler could re-write this as:

	if (array->index_mask != 0xffffffff)
		index &= array->index_mask;
	return this_cpu_ptr(array->pptrs[index]);

... which would allow an unmasked index to be used in speculated paths.

Similar cases could occur with some CPU implementations. For example, HW
value-prediction could result in the use of an all-ones mask under
speculation.

I think that we may need to be able to provide an arch-specific
pointer sanitization sequence (though we could certainly have masking as
the default).

I have a rough idea as to how that could be plumbed into the JIT. First
I need to verify the sequence I have in mind for arm/arm64 is
sufficient.

Thanks,
Mark.

^ permalink raw reply

* [PATCH iproute2 v3 6/7] gre6/tunnel: Output flowlabel after tclass
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

To follow ip6tnl output print flowlabel after tclass
in link_gre6.c.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c |   24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 8014207..55bd1fb 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -444,18 +444,6 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
-	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-		print_bool(PRINT_ANY,
-			   "ip6_tnl_f_use_orig_flowlabel",
-			   "flowlabel inherit ",
-			   true);
-	} else if (tb[IFLA_GRE_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
-
-		snprintf(s2, sizeof(s2), "0x%05x", val);
-		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
-	}
-
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
@@ -468,6 +456,18 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
+	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
+		print_bool(PRINT_ANY,
+			   "ip6_tnl_f_use_orig_flowlabel",
+			   "flowlabel inherit ",
+			   true);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
+
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
+	}
+
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 5/7] ip6/tunnel: Unify encap_limit printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

Use %u format specifier to print it in link_gre6.c and
make code more readable.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   11 ++++-------
 ip/link_ip6tnl.c |   12 ++++++------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 1205946..8014207 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -433,18 +433,15 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			print_int(PRINT_JSON, "ttl", NULL, ttl);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
-		int encap_limit = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
+	} else if (tb[IFLA_GRE_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_GRE_ENCAP_LIMIT]);
 
-		print_int(PRINT_ANY,
-			  "encap_limit",
-			  "encaplimit %d ",
-			  encap_limit);
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 7000056..379eb33 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -387,16 +387,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			print_uint(PRINT_ANY, "link_index", "dev %u ", link);
 	}
 
-	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT)
+	if (flags & IP6_TNL_F_IGN_ENCAP_LIMIT) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_ign_encap_limit",
 			   "encaplimit none ",
 			   true);
-	else if (tb[IFLA_IPTUN_ENCAP_LIMIT])
-		print_uint(PRINT_ANY,
-			   "encap_limit",
-			   "encaplimit %u ",
-			   rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]));
+	} else if (tb[IFLA_IPTUN_ENCAP_LIMIT]) {
+		__u8 val = rta_getattr_u8(tb[IFLA_IPTUN_ENCAP_LIMIT]);
+
+		print_uint(PRINT_ANY, "encap_limit", "encaplimit %u ", val);
+	}
 
 	if (tb[IFLA_IPTUN_TTL])
 		print_uint(PRINT_ANY,
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 4/7] ip6/tunnel: Unify flowlabel printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

Use @s2 buffer to store string representation of
flowlabel and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

Omit flowlabel from output if no flowinfo attribute
is given and IP6_TNL_F_USE_ORIG_FLOWLABEL isn't set.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   15 ++++-----------
 ip/link_ip6tnl.c |   14 ++++----------
 2 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index a02dd4a..1205946 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -452,18 +452,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-		} else {
-			fprintf(f, "flowlabel 0x%05x ",
-				ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 05322fd..7000056 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -421,17 +421,11 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "ip6_tnl_f_use_orig_flowlabel",
 			   "flowlabel inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
-				 ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-			print_string(PRINT_JSON, "flowlabel", NULL, b1);
-		} else {
-			printf("flowlabel 0x%05x ",
-			       ntohl(flowinfo & IP6_FLOWINFO_FLOWLABEL));
-		}
+		snprintf(s2, sizeof(s2), "0x%05x", val);
+		print_string(PRINT_ANY, "flowlabel", "flowlabel %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 3/7] ip6/tunnel: Unify tclass printing
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

Use @s2 buffer to store string representation of
tclass and get rid of extra SPRINT_BUF(): no
need to preserve @s2 contents for later.

Use print_string(PRINT_ANY, ...) with prepared by
snprintf() string for both PRINT_JSON and PRINT_FP
cases.

While there use __u32 for flowinfo in link_gre6.c
and check for IFLA_GRE_FLOWINFO attribute presense.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |   16 +++++-----------
 ip/link_ip6tnl.c |   16 +++++-----------
 2 files changed, 10 insertions(+), 22 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 87c313c..a02dd4a 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -380,7 +380,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 	unsigned int iflags = 0;
 	unsigned int oflags = 0;
 	unsigned int flags = 0;
-	unsigned int flowinfo = 0;
+	__u32 flowinfo = 0;
 	struct in6_addr in6_addr_any = IN6ADDR_ANY_INIT;
 
 	if (!tb)
@@ -471,17 +471,11 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	} else {
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
+	} else if (tb[IFLA_GRE_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-			snprintf(b1, sizeof(b1), "0x%02x",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			fprintf(f, "tclass 0x%02x ",
-				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index e084975..05322fd 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -404,22 +404,16 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			   "hoplimit %u ",
 			   rta_getattr_u8(tb[IFLA_IPTUN_TTL]));
 
-	if (flags & IP6_TNL_F_USE_ORIG_TCLASS)
+	if (flags & IP6_TNL_F_USE_ORIG_TCLASS) {
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_use_orig_tclass",
 			   "tclass inherit ",
 			   true);
-	else if (tb[IFLA_IPTUN_FLOWINFO]) {
-		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS);
+	} else if (tb[IFLA_IPTUN_FLOWINFO]) {
+		__u32 val = ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20;
 
-		if (is_json_context()) {
-			SPRINT_BUF(b1);
-
-			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "tclass", NULL, b1);
-		} else {
-			printf("tclass 0x%02x ", (__u8)(val >> 20));
-		}
+		snprintf(s2, sizeof(s2), "0x%02x", val);
+		print_string(PRINT_ANY, "tclass", "tclass %s ", s2);
 	}
 
 	if (flags & IP6_TNL_F_USE_ORIG_FLOWLABEL) {
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 2/7] ip6tnl/tunnel: Do not print obscure flowinfo
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

It is implementation internal and main purpose
of printing it seems debugging.

Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_ip6tnl.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 8e84ed0..e084975 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -440,16 +440,6 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 		}
 	}
 
-	if (is_json_context()) {
-		SPRINT_BUF(flwinfo);
-
-		snprintf(flwinfo, sizeof(flwinfo), "0x%08x", ntohl(flowinfo));
-		print_string(PRINT_JSON, "flowinfo", NULL, flwinfo);
-	} else {
-		printf("(flowinfo 0x%08x) ", ntohl(flowinfo));
-
-	}
-
 	if (flags & IP6_TNL_F_RCV_DSCP_COPY)
 		print_bool(PRINT_ANY,
 			   "ip6_tnl_f_rcv_dscp_copy",
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 1/7] ip6/tunnel: Fix tclass output
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev
In-Reply-To: <1515431072-29128-1-git-send-email-serhe.popovych@gmail.com>

In link_gre6.c it seems copy paste error: tclass is 8 bits,
not 20 as flowlabel.

In link_iptnl.c rename "flowinfo_tclass" to "tclass" as it
correct name since flowinfo is implementation internal name
used to label combined within u32 attribute tclass and
flowlabel.

Fixes: 1facc1c61c07 ("ip: link_ip6tnl.c: add json output support")
Fixes: 2e706e12d9b0 ("Merge branch 'master' into net-next")
Signed-off-by: Serhey Popovych <serhe.popovych@gmail.com>
---
 ip/link_gre6.c   |    2 +-
 ip/link_ip6tnl.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ip/link_gre6.c b/ip/link_gre6.c
index 7ae4b49..87c313c 100644
--- a/ip/link_gre6.c
+++ b/ip/link_gre6.c
@@ -475,7 +475,7 @@ static void gre_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb[])
 		if (is_json_context()) {
 			SPRINT_BUF(b1);
 
-			snprintf(b1, sizeof(b1), "0x%05x",
+			snprintf(b1, sizeof(b1), "0x%02x",
 				 ntohl(flowinfo & IP6_FLOWINFO_TCLASS) >> 20);
 			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
diff --git a/ip/link_ip6tnl.c b/ip/link_ip6tnl.c
index 84205b1..8e84ed0 100644
--- a/ip/link_ip6tnl.c
+++ b/ip/link_ip6tnl.c
@@ -416,7 +416,7 @@ static void ip6tunnel_print_opt(struct link_util *lu, FILE *f, struct rtattr *tb
 			SPRINT_BUF(b1);
 
 			snprintf(b1, sizeof(b1), "0x%02x", (__u8)(val >> 20));
-			print_string(PRINT_JSON, "flowinfo_tclass", NULL, b1);
+			print_string(PRINT_JSON, "tclass", NULL, b1);
 		} else {
 			printf("tclass 0x%02x ", (__u8)(val >> 20));
 		}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH iproute2 v3 0/7] ip6/tunnel: Unify tclass, flowlabel and encap_limit output
From: Serhey Popovych @ 2018-01-08 17:04 UTC (permalink / raw)
  To: netdev

With this series I want to improve printing of tclass,
flowlabel and encapsulation limit (encap_limit) options.

Everything within this series is open for your comments,
suggestions and criticism.

See individual patch description message for details.

Thanks,
Serhii

Serhey Popovych (7):
  ip6/tunnel: Fix tclass output
  ip6tnl/tunnel: Do not print obscure flowinfo
  ip6/tunnel: Unify tclass printing
  ip6/tunnel: Unify flowlabel printing
  ip6/tunnel: Unify encap_limit printing
  gre6/tunnel: Output flowlabel after tclass
  ip6tnl/tunnel: Output hoplimit before encapsulation limit

 ip/link_gre6.c   |   54 ++++++++++++++++------------------------------
 ip/link_ip6tnl.c |   63 ++++++++++++++++++------------------------------------
 2 files changed, 40 insertions(+), 77 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* Re: b53 tags on bpi-r1 (bcm53125)
From: Jochen Friedrich @ 2018-01-08 16:51 UTC (permalink / raw)
  To: Florian Fainelli; +Cc: netdev
In-Reply-To: <08f78158-3072-6929-995f-eda622833256@gmail.com>

Hi Florian,

Am 04.01.2018 um 05:49 schrieb Florian Fainelli:
> On 12/29/2017 07:22 PM, Florian Fainelli wrote:
>> Le 12/29/17 à 13:56, Florian Fainelli a écrit :
>>> Le 12/29/17 à 12:21, Florian Fainelli a écrit :
>>>> Hi Jochen,
>>>>
>>>> Le 12/18/17 à 02:44, Jochen Friedrich a écrit :
>>>>> Hi Florian,
>>>>>
>>>>> unfortunately, this doesn't make any difference.
>>>>>
>>>>> Just out of curiosity, BPI-R1 has pull-down resistors on LED6 and 7
>>>>> (MII_MODE0/1). However, the public available 53125U sheet doesn't
>>>>> document these pins:
>>>>>
>>>>> LED[6] IMP_MODE[0] Pull-up Active low (since IMP Mode is not used)
>>>>> LED[7] IMP_MODE[1] Pull-up Active low (since IMP Mode is not used)
>>>>>
>>>>> Is this MII mode maybe incompatible with Broadcom tags?
>>>> AFAICT, it should not, this is largely independent from enabling
>>>> Broadcom tags.
>>>>
>>>> I have now reproduced this on my BPI-R1 as well and am looking at what
>>>> might be going wrong.
>>> OK, so I have been able to find out what was going on. On BCM53125 and
>>> earlier switches, we need to do a couple of things for Broadcom tags to
>>> be usable:
>>>
>>> - turn on managed mode (SM_SW_FWD_MODE)
>>> - configure Port 8 for IMP mode (B53_GLOBAL_CONFIG, setting bit
>>> GC_FRM_MGMT_PORT_MII)
>>>
>>> After doing that, I can now see the correct outgoing packets on my host,
>>> however, the replies don't seem to be delivered to the per-port DSA
>>> network devices, and I suspect it's because of stmmac, so I am
>>> investigating this now.
>>>
>> So stmmac was indeed part of the problem. I had to clear the
>> GMAC_CONTROL_ACS bit in GMAC_CORE_INIT in order to allow stmmac to
>> properly receive packets, otherwise, packets were truncated to 8 bytes
>> on reception which I assume is because the Broadcom tags may look like
>> some sort of weird LLC/SNAP packet which was confusing the hardware.
>> This is all working correctly now after a bunch of changes that I will
>> submit in the next few days.
>>
>> Preliminary patches available here:
>>
>> https://github.com/ffainelli/linux/commits/stmmac-fixes
>>
>> Thank you very much for your patience!
> Turning on managed mode has some other effects, like how multicast
> traffic needs to be handled and how to resolve ARL misses, which means
> that the changes are too invasive for the upcoming release since we
> would be essentially adding multicast support through a number of code
> paths (including core DSA).
>
> I will submit a simple change for now which does not turn on Broadcom
> tags on switches that require managed mode (5395, 97, 98, 53125 and
> friends) and target enabling Broadcom tags for these models for net-next.
>
> Thanks

Thanks. That's the best approach for now :-)

^ permalink raw reply

* Re: [PATCH bpf] selftests/bpf: fix test_align
From: Edward Cree @ 2018-01-08 16:38 UTC (permalink / raw)
  To: Alexei Starovoitov, David S . Miller; +Cc: Daniel Borkmann, netdev
In-Reply-To: <20180105230200.2183754-1-ast@fb.com>

On 05/01/18 23:02, Alexei Starovoitov wrote:
> since commit 82abbf8d2fc4 the verifier rejects the bit-wise
> arithmetic on pointers earlier.
> The test 'dubious pointer arithmetic' now has less output to match on.
> Adjust it.
>
> Fixes: 82abbf8d2fc4 ("bpf: do not allow root to mangle valid pointers")
> Reported-by: kernel test robot <xiaolong.ye@intel.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  tools/testing/selftests/bpf/test_align.c | 22 +---------------------
>  1 file changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
> index 8591c89c0828..471bbbdb94db 100644
> --- a/tools/testing/selftests/bpf/test_align.c
> +++ b/tools/testing/selftests/bpf/test_align.c
> @@ -474,27 +474,7 @@ static struct bpf_align_test tests[] = {
>  		.result = REJECT,
>  		.matches = {
>  			{4, "R5=pkt(id=0,off=0,r=0,imm=0)"},
> -			/* ptr & 0x40 == either 0 or 0x40 */
> -			{5, "R5=inv(id=0,umax_value=64,var_off=(0x0; 0x40))"},
> -			/* ptr << 2 == unknown, (4n) */
> -			{7, "R5=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
> -			/* (4n) + 14 == (4n+2).  We blow our bounds, because
> -			 * the add could overflow.
> -			 */
> -			{8, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
> -			/* Checked s>=0 */
> -			{10, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> -			/* packet pointer + nonnegative (4n+2) */
> -			{12, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> -			{14, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> -			/* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
> -			 * We checked the bounds, but it might have been able
> -			 * to overflow if the packet pointer started in the
> -			 * upper half of the address space.
> -			 * So we did not get a 'range' on R6, and the access
> -			 * attempt will fail.
> -			 */
> -			{16, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
> +			/* R5 bitwise operator &= on pointer prohibited */
>  		}
>  	},
>  	{
Rather than neutering this test, we should change it to keep the part where
 it tests that a large pkt_ptr offset prevents us getting a reg->range.
Specifically, in this test we have
    r2 = pkt
    r5 = large unknown scalar
    r6 = r2 + r5
    r4 = r6 + 4
Then we check r4 < pkt_end, which normally would give r6->range = 4, but in
 this case must not do so since r6 could be (u64)(-2) in which case r4 = 2
 < pkt_end despite r6 not pointing into the packet.
AFAICT there is not other coverage of this case in test_align, and I don't
 recall such a test being in test_verifier either.  So please instead replace
 the insns that do prohibited ops on pointers with some other way of creating
 a large unknown scalar, and keep the rest of the test case intact.

-Ed

^ permalink raw reply

* Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post
From: Larry Finger @ 2018-01-08 16:31 UTC (permalink / raw)
  To: Kalle Valo, Jia-Ju Bai
  Cc: kstewart, johannes.berg, tiwai, gregkh, linux-wireless,
	linux-kernel, andrew.zaborowski, b43-dev, netdev, colin.king
In-Reply-To: <20180108162128.6D37560B1D@smtp.codeaurora.org>

On 01/08/2018 10:21 AM, Kalle Valo wrote:
> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> 
>> b43_radio_2057_init_post is not called in an interrupt handler
>> nor holding a spinlock.
>> The function mdelay in it can be replaced with msleep, to reduce busy wait.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> 
> You submitted an identical patch a week earlier:
> 
> https://patchwork.kernel.org/patch/10137671/
> 
> How is this different? Also always add version number to the patch so that the
> maintainers can follow the changes easily:
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing
> 
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing

I had negative comments on one of those due to the possibility of msleep(2) 
extending as long as 20 msec. Until the author, or someone else, can test that 
this is OK, then the mdelay(2) can only be replaced with usleep_range(2000, 3000).

NACK for both.

Larry

^ permalink raw reply

* Re: Aw: Re: dvb usb issues since kernel 4.9
From: Alan Stern @ 2018-01-08 16:31 UTC (permalink / raw)
  To: Josef Griebichler
  Cc: Mauro Carvalho Chehab, Greg Kroah-Hartman, linux-usb,
	Eric Dumazet, Rik van Riel, Paolo Abeni, Hannes Frederic Sowa,
	Jesper Dangaard Brouer, linux-kernel, netdev, Jonathan Corbet,
	LMML, Peter Zijlstra, David Miller, torvalds
In-Reply-To: <trinity-c7ec7cbd-a186-4a2a-bcb6-cce8993d6a90-1515428770628@3c-app-gmx-bs32>

On Mon, 8 Jan 2018, Josef Griebichler wrote:

> Hi Maro,
> 
> I tried your mentioned patch but unfortunately no real improvement for me.
> dmesg http://ix.io/DOg
> tvheadend service log http://ix.io/DOi
> Errors during recording are still there.
> Errors increase if there is additional tcp load on raspberry.
> 
> Unfortunately there's no usbmon or tshark on libreelec so I can't provide further logs.

Can you try running the same test on an x86_64 system?

Alan Stern

^ permalink raw reply

* Re: [PATCH v5 1/3] clocksource/drivers/atcpit100: Add andestech atcpit100 timer
From: Arnd Bergmann @ 2018-01-08 16:30 UTC (permalink / raw)
  To: Daniel Lezcano
  Cc: Greentime Hu, Greentime, Rick Chen, Rick Chen,
	Linux Kernel Mailing List, Linus Walleij, linux-arch,
	Thomas Gleixner, Jason Cooper, Marc Zyngier, Rob Herring, netdev,
	Vincent Chen, DTML, Al Viro, David Howells, Will Deacon,
	linux-serial, John Stultz
In-Reply-To: <78dc7813-524d-c108-0e3d-516f8f4dabfe@linaro.org>

On Mon, Jan 8, 2018 at 5:08 PM, Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
> On 08/01/2018 16:26, Arnd Bergmann wrote:
>> On Fri, Jan 5, 2018 at 10:31 AM, Daniel Lezcano
>> <daniel.lezcano@linaro.org> wrote:

>>>
>>> etc ...
>>
>> I'd actually prefer to not do it for ARM either: Most other subsystems
>> don't do that, and I don't see a strong reason why clocksource should
>> be special here.
>
> The majority doing the opposite does not mean it is right.
>
> Do you know which clock belongs to which board ? Who will unselect a
> clock ? I'm pretty sure nobody. Everyone relies on the platform Kconfig
> and expect it to select the right drivers.

The point is that there is no platform specific Kconfig that could select
it, as the concept doesn't make a lot of sense here.

If you require the driver to always be selected by the
architecture code, it adds a little bit of bloat on systems
that don't need it. This is possible, but I think it's preferable
to give users a way of tuning a kernel for a particular chip.

> We don't expect the hackers to have a deep knowledge of the hardware and
> the driver dependencies. It is very convenient to not care about that
> and let the platform's Kconfig to select the right drivers.
>
> And that is the behavior I would like to keep.

I'm not worried about the driver bloating the kernel here when it
isn't disabled, this is no different from enabling the USB controller
by default for a board that doesn't have a USB connector, or
enabling ten different pinctrl drivers for all members of a chip
family even though you know which particular chip you are
running on.

>> Selecting 'TIMER_OF' from the individual drivers that need it (as you
>> suggest) makes sense, but I think for ARM we treat SoC families
>> as a bit too special, in the end they are for the most part collections
>> of individual hardware blocks that may or may not be present on
>> some chip.
>>
>> In case of risc-v and nds32, I expect that the separation will be
>> even less visibile in the hardware, as a typical model here is
>> that one company designs SoCs for multiple customers that each
>> have different requirements. Some of them may have one
>> timer and some have another timer or multiple timers, but there
>> is no strict separation between SoC families as I understand.
>> Here we'd be better off not having a per-SoC Kconfig option at
>> all, just a generic defconfig that enables all the drivers that might
>> be used, and integrators can have a defconfig file that only
>> enables the stuff they actually use on a given chip.
>
> Yes, the result is the same, the option is not showed in the menu.
>
> However, I can understand it could be interesting to have the ability to
> unselect a driver for experts.
>
> I'm wondering if we can create a bool_expert which shows up only when
> CONFIG_EXPERT is set.

Having a dependency on CONFIG_EXPERT means that a more
users will end up having to set that for a shipping system. It's
something we can do (even without a special Kconfig keyword),
but it should be used carefully for stuff that 99% of the users want
to enable.

Why not just:

config CLKSRC_ATCPIT100
       bool "Clocksource for AE3XX platform"
       depends on NDS32 || COMPILE_TEST
       depends on HAS_IOMEM
       default NDS32
       help
         This option enables support for the Andestech AE3XX platform timers.

That way, it's simply enabled on NDS32 by default, but just as easy
to disable for systems that don't need it.

      Arnd

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox