Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next 2/2] bpf: reuse tc bpf prologue for sk skb progs
From: Alexei Starovoitov @ 2017-08-17 16:12 UTC (permalink / raw)
  To: John Fastabend, Daniel Borkmann, davem; +Cc: netdev
In-Reply-To: <5995BEB1.8020509@gmail.com>

On 8/17/17 9:05 AM, John Fastabend wrote:
> On 08/17/2017 08:22 AM, Daniel Borkmann wrote:
>> Given both program types are effecitvely doing the same in the
>> prologue, just reuse the one that we had for tc and only adapt
>> to the corresponding drop verdict value. That way, we don't need
>> to have the duplicate from 8a31db561566 ("bpf: add access to sock
>> fields and pkt data from sk_skb programs") to maintain.
>>
>> Reported-by: Alexei Starovoitov <ast@kernel.org>
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>>  net/core/filter.c | 47 ++++++++++-------------------------------------
>>  1 file changed, 10 insertions(+), 37 deletions(-)
>>
>
> Nice clean-up, Thanks.
>
> Acked-by: John Fastabend <john.fastabend@gmail.com>
>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH 1/6] bridge: learn dst metadata in FDB
From: David Lamparter @ 2017-08-17 16:16 UTC (permalink / raw)
  To: Nikolay Aleksandrov
  Cc: David Lamparter, netdev, amine.kherbouche, roopa, stephen,
	bridge@lists.linux-foundation.org
In-Reply-To: <1fbaa432-6241-fad0-5407-8b87abb965ae@cumulusnetworks.com>

On Wed, Aug 16, 2017 at 11:38:06PM +0300, Nikolay Aleksandrov wrote:
> On 16/08/17 20:01, David Lamparter wrote:
> > This implements holding dst metadata information in the bridge layer,
> > but only for unicast entries in the MAC table.  Multicast is still left
> > to design and implement.
> > 
> > Signed-off-by: David Lamparter <equinox@diac24.net>
> > ---
> 
> Hi David,
> Sorry but I do not agree with this change, adding a special case for VPLS

To prove that this is not a special case for VPLS, I have attached a
patch for GRETAP multicast+unicast learning below.

It's just 24(!) lines added to get functionality similar to "basic
VXLAN" (i.e. multicast with dataplane learning.)


-David

---
From: David Lamparter <equinox@diac24.net>
Date: Thu, 17 Aug 2017 18:11:16 +0200
Subject: [PATCH] gretap: support multicast + unicast learning

This enables using an IPv4 multicast destination for gretap and enables
learning unicast destinations through the bridge fdb.

Signed-off-by: David Lamparter <equinox@diac24.net>
---
 net/ipv4/ip_gre.c    | 27 +++++++++++++++++++++++----
 net/ipv4/ip_tunnel.c |  1 +
 2 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
index 7a7829e839c2..e58f8ccb2c87 100644
--- a/net/ipv4/ip_gre.c
+++ b/net/ipv4/ip_gre.c
@@ -266,7 +266,8 @@ static int __ipgre_rcv(struct sk_buff *skb, const struct tnl_ptk_info *tpi,
 			skb_pop_mac_header(skb);
 		else
 			skb_reset_mac_header(skb);
-		if (tunnel->collect_md) {
+		if (tunnel->collect_md
+		    || ipv4_is_multicast(tunnel->parms.iph.daddr)) {
 			__be16 flags;
 			__be64 tun_id;
 
@@ -379,7 +380,7 @@ static struct rtable *gre_get_rt(struct sk_buff *skb,
 static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 			__be16 proto)
 {
-	struct ip_tunnel_info *tun_info;
+	struct ip_tunnel_info *tun_info, flipped;
 	const struct ip_tunnel_key *key;
 	struct rtable *rt = NULL;
 	struct flowi4 fl;
@@ -390,10 +391,22 @@ static void gre_fb_xmit(struct sk_buff *skb, struct net_device *dev,
 	int err;
 
 	tun_info = skb_tunnel_info(skb);
-	if (unlikely(!tun_info || !(tun_info->mode & IP_TUNNEL_INFO_TX) ||
+	if (unlikely(!tun_info ||
 		     ip_tunnel_info_af(tun_info) != AF_INET))
 		goto err_free_skb;
 
+	if (!(tun_info->mode & IP_TUNNEL_INFO_TX)) {
+		struct ip_tunnel *tunnel = netdev_priv(dev);
+
+		flipped = *tun_info;
+		flipped.mode |= IP_TUNNEL_INFO_TX;
+		flipped.key.u.ipv4.dst = tun_info->key.u.ipv4.src;
+		flipped.key.u.ipv4.src = tunnel->parms.iph.saddr;
+		flipped.key.tp_src = tun_info->key.tp_dst;
+		flipped.key.tp_dst = tun_info->key.tp_src;
+		tun_info = &flipped;
+	}
+
 	key = &tun_info->key;
 	use_cache = ip_tunnel_dst_cache_usable(skb, tun_info);
 	if (use_cache)
@@ -507,8 +520,9 @@ static netdev_tx_t gre_tap_xmit(struct sk_buff *skb,
 				struct net_device *dev)
 {
 	struct ip_tunnel *tunnel = netdev_priv(dev);
+	struct ip_tunnel_info *tun_info = skb_tunnel_info(skb);
 
-	if (tunnel->collect_md) {
+	if (tunnel->collect_md || tun_info) {
 		gre_fb_xmit(skb, dev, htons(ETH_P_TEB));
 		return NETDEV_TX_OK;
 	}
@@ -933,6 +947,7 @@ static int gre_tap_init(struct net_device *dev)
 {
 	__gre_tunnel_init(dev);
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
+	netif_keep_dst(dev);
 
 	return ip_tunnel_init(dev);
 }
@@ -940,6 +955,10 @@ static int gre_tap_init(struct net_device *dev)
 static const struct net_device_ops gre_tap_netdev_ops = {
 	.ndo_init		= gre_tap_init,
 	.ndo_uninit		= ip_tunnel_uninit,
+#ifdef CONFIG_NET_IPGRE_BROADCAST
+	.ndo_open		= ipgre_open,
+	.ndo_stop		= ipgre_close,
+#endif
 	.ndo_start_xmit		= gre_tap_xmit,
 	.ndo_set_mac_address 	= eth_mac_addr,
 	.ndo_validate_addr	= eth_validate_addr,
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 129d1a3616f8..451c11fc9ae5 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -140,6 +140,7 @@ struct ip_tunnel *ip_tunnel_lookup(struct ip_tunnel_net *itn,
 
 	hlist_for_each_entry_rcu(t, head, hash_node) {
 		if ((local != t->parms.iph.saddr || t->parms.iph.daddr != 0) &&
+		    (local != t->parms.iph.saddr || !ipv4_is_multicast(t->parms.iph.daddr)) &&
 		    (local != t->parms.iph.daddr || !ipv4_is_multicast(local)))
 			continue;
 
-- 
2.13.0

^ permalink raw reply related

* [PATCH 0/2] xdp: adjust xdp redirect tracepoint
From: Jesper Dangaard Brouer @ 2017-08-17 16:22 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jesper Dangaard Brouer

Working on streamlining the tracepoints for XDP.  The eBPF programs
and XDP have no flow-control or queueing.  Investigating using
tracepoint to provide a feedback on XDP_REDIRECT xmit overflow events.

---

Jesper Dangaard Brouer (2):
      ixgbe: change ndo_xdp_xmit return code on xmit errors
      xdp: adjust xdp redirect tracepoint to include return error code


 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 include/trace/events/xdp.h                    |   11 +++++++----
 net/core/filter.c                             |   19 ++++++++++++-------
 3 files changed, 20 insertions(+), 12 deletions(-)

^ permalink raw reply

* [PATCH 1/2] ixgbe: change ndo_xdp_xmit return code on xmit errors
From: Jesper Dangaard Brouer @ 2017-08-17 16:22 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jesper Dangaard Brouer
In-Reply-To: <150298692691.6608.11908184719252996949.stgit@firesoul>

Use errno -ENOSPC ("No space left on device") when the XDP xmit
have no space left on the TX ring buffer, instead of -ENOMEM.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index f9fd8d8f1bef..017cd5e85c97 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -9860,7 +9860,7 @@ static int ixgbe_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
 
 	err = ixgbe_xmit_xdp_ring(adapter, xdp);
 	if (err != IXGBE_XDP_TX)
-		return -ENOMEM;
+		return -ENOSPC;
 
 	return 0;
 }

^ permalink raw reply related

* [PATCH 2/2] xdp: adjust xdp redirect tracepoint to include return error code
From: Jesper Dangaard Brouer @ 2017-08-17 16:22 UTC (permalink / raw)
  To: netdev; +Cc: John Fastabend, Jesper Dangaard Brouer
In-Reply-To: <150298692691.6608.11908184719252996949.stgit@firesoul>

The return error code need to be included in the tracepoint
xdp:xdp_redirect, else its not possible to distinguish successful or
failed XDP_REDIRECT transmits.

XDP have no queuing mechanism. Thus, it is fairly easily to overrun a
NIC transmit queue.  The eBPF program invoking helpers (bpf_redirect
or bpf_redirect_map) to redirect a packet doesn't get any feedback
whether the packet was actually transmitted.

Info on failed transmits in the tracepoint xdp:xdp_redirect, is
interesting as this opens for providing a feedback-loop to the
receiving XDP program.

Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
---
 include/trace/events/xdp.h |   11 +++++++----
 net/core/filter.c          |   19 ++++++++++++-------
 2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 7b1eb7b4be41..0e42e69f773b 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -53,15 +53,16 @@ TRACE_EVENT(xdp_redirect,
 
 	TP_PROTO(const struct net_device *from,
 		 const struct net_device *to,
-		 const struct bpf_prog *xdp, u32 act),
+		 const struct bpf_prog *xdp, u32 act, int err),
 
-	TP_ARGS(from, to, xdp, act),
+	TP_ARGS(from, to, xdp, act, err),
 
 	TP_STRUCT__entry(
 		__string(name_from, from->name)
 		__string(name_to, to->name)
 		__array(u8, prog_tag, 8)
 		__field(u32, act)
+		__field(int, err)
 	),
 
 	TP_fast_assign(
@@ -70,12 +71,14 @@ TRACE_EVENT(xdp_redirect,
 		__assign_str(name_from, from->name);
 		__assign_str(name_to, to->name);
 		__entry->act = act;
+		__entry->err = err;
 	),
 
-	TP_printk("prog=%s from=%s to=%s action=%s",
+	TP_printk("prog=%s from=%s to=%s action=%s err=%d",
 		  __print_hex_str(__entry->prog_tag, 8),
 		  __get_str(name_from), __get_str(name_to),
-		  __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB))
+		  __print_symbolic(__entry->act, __XDP_ACT_SYM_TAB),
+		  __entry->err)
 );
 #endif /* _TRACE_XDP_H */
 
diff --git a/net/core/filter.c b/net/core/filter.c
index 5afe3ac191ec..70c9631da7f2 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2496,14 +2496,16 @@ int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
 	struct bpf_map *map = ri->map;
 	u32 index = ri->ifindex;
 	struct net_device *fwd;
-	int err = -EINVAL;
+	int err;
 
 	ri->ifindex = 0;
 	ri->map = NULL;
 
 	fwd = __dev_map_lookup_elem(map, index);
-	if (!fwd)
+	if (!fwd) {
+		err = -EINVAL;
 		goto out;
+	}
 
 	if (ri->map_to_flush && (ri->map_to_flush != map))
 		xdp_do_flush_map();
@@ -2513,7 +2515,7 @@ int xdp_do_redirect_map(struct net_device *dev, struct xdp_buff *xdp,
 		ri->map_to_flush = map;
 
 out:
-	trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT);
+	trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err);
 	return err;
 }
 
@@ -2523,6 +2525,7 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 	struct redirect_info *ri = this_cpu_ptr(&redirect_info);
 	struct net_device *fwd;
 	u32 index = ri->ifindex;
+	int err;
 
 	if (ri->map)
 		return xdp_do_redirect_map(dev, xdp, xdp_prog);
@@ -2532,12 +2535,14 @@ int xdp_do_redirect(struct net_device *dev, struct xdp_buff *xdp,
 	ri->map = NULL;
 	if (unlikely(!fwd)) {
 		bpf_warn_invalid_xdp_redirect(index);
-		return -EINVAL;
+		err = -EINVAL;
+		goto out;
 	}
 
-	trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT);
-
-	return __bpf_tx_xdp(fwd, NULL, xdp, 0);
+	err = __bpf_tx_xdp(fwd, NULL, xdp, 0);
+out:
+	trace_xdp_redirect(dev, fwd, xdp_prog, XDP_REDIRECT, err);
+	return err;
 }
 EXPORT_SYMBOL_GPL(xdp_do_redirect);
 

^ permalink raw reply related

* Re: [PATCH net] datagram: When peeking datagrams with offset < 0 don't skip empty skbs
From: Willem de Bruijn @ 2017-08-17 16:45 UTC (permalink / raw)
  To: Matthew Dawson; +Cc: Paolo Abeni, Network Development, Macieira, Thiago
In-Reply-To: <2020045.ridbXvZZ6f@ring00>

>> @@ -169,14 +169,20 @@ struct sk_buff *__skb_try_recv_from_queue(struct sock
>> *sk, int *peeked, int *off, int *err, struct sk_buff **last)
>>  {
>> +       bool peek_at_off = false;
>>         struct sk_buff *skb;
>> -       int _off = *off;
>> +       int _off = 0;
>> +
>> +       if (flags & MSG_PEEK && (*off) >= 0) {
>> +               peek_at_off = true;
>> +               _off = *off;
>> +       }
>>
>>         *last = queue->prev;
>>         skb_queue_walk(queue, skb) {
>>                 if (flags & MSG_PEEK) {
>> -                       if (_off >= skb->len && (skb->len || _off ||
>> -                                                skb->peeked)) {
>> +                       if (peek_at_off && _off >= skb->len &&
>> +                           (skb->len || _off || skb->peeked)) {
>                                 ^ I'm pretty sure we can remove this check
> (that skb->len is not zero) in this if statement.  If _off is zero, then skb-
>>len must also be zero (since _off >= skb->len, if _off is 0, skb->len <= 0.
> If skb->len can't be negative, then skb->len <= 0 => skb->len == 0).  If _off
> is not zero, then checking skb->len is redundant.

Good point.

>>                                 _off -= skb->len;
>>                                 continue;
>>                         }
> Is this queued to go in already? Or can I help by updating my patch with what
> was discussed here?  I can do that today if wanted.

It isn't. Please do if no one else has additional comments.

^ permalink raw reply

* Re: [PATCH net-next] net: ipv6: put host and anycast routes on device with address
From: David Ahern @ 2017-08-17 16:56 UTC (permalink / raw)
  To: netdev, David Miller; +Cc: yoshfuji, hannes
In-Reply-To: <1502923076-12292-1-git-send-email-dsahern@gmail.com>

On 8/16/17 4:37 PM, David Ahern wrote:
> One nagging difference between ipv4 and ipv6 is host routes for ipv6
> addresses are installed using the loopback device or VRF / L3 Master
> device. e.g.,
> 
>     2001:db8:1::/120 dev veth0 proto kernel metric 256 pref medium
>     local 2001:db8:1::1 dev lo table local proto kernel metric 0 pref medium
> 
> Using the loopback device is convenient -- necessary for local tx, but
> has some nasty side effects, most notably setting the 'lo' device down
> causes all host routes for all local IPv6 address to be removed from the
> FIB and completely breaks IPv6 networking across all interfaces.
> 
> This patch puts FIB entries for IPv6 routes against the device. This
> simplifies the routes in the FIB, for example by making dst->dev and
> rt6i_idev->dev the same (a future patch can look at removing the device
> reference taken for rt6i_idev for FIB entries). For example:
> 
>     $ ip -6 r ls table all | grep veth1
>     2001:db8:99::/120 dev veth1 proto kernel metric 256 pref medium
>     anycast 2001:db8:99:: dev veth1 table local proto kernel metric 0 pref medium
>     local 2001:db8:99::1 dev veth1 table local proto kernel metric 0 pref medium
> 
> When copies are made on FIB lookups, the cloned route has dst->dev
> set to loopback (or the L3 master device). This is needed for the
> local Tx of packets to local addresses.
> 
> With fib entries allocated against the real network device, the addrconf
> code that reinserts host routes on admin up of 'lo' is no longer needed.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  net/ipv6/addrconf.c | 42 ------------------------------------------
>  net/ipv6/route.c    | 46 ++++++++++++++++++++++++++++++++++------------
>  2 files changed, 34 insertions(+), 54 deletions(-)
>

DaveM: please drop this one. I found a use case that is failing: UDP
packets to a local linklocal address with no server are not getting the
ICMP unreachable. Will send a v2 when tests complete.

^ permalink raw reply

* Re: [PATCH] PCI: Allow PCI express root ports to find themselves
From: Bjorn Helgaas @ 2017-08-17 16:58 UTC (permalink / raw)
  To: Thierry Reding
  Cc: David Miller, Bjorn Helgaas, Ding Tianhong, Michael Ellerman,
	mark.rutland, gabriele.paoloni, asit.k.mallick, catalin.marinas,
	will.deacon, linuxarm, alexander.duyck, ashok.raj, eric.dumazet,
	jeffrey.t.kirsher, linux-pci, ganeshgr, Bob.Shaw, leedom,
	patrick.j.cramer, werner, linux-arm-kernel, amira, netdev,
	linux-kernel, David.Laight, Suravee.Suthikulpanit, robin.murphy,
	l.stach
In-Reply-To: <20170817110614.11454-1-thierry.reding@gmail.com>

On Thu, Aug 17, 2017 at 01:06:14PM +0200, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> If the pci_find_pcie_root_port() function is called on a root port
> itself, return the root port rather than NULL.
> 
> This effectively reverts commit 0e405232871d6 ("PCI: fix oops when
> try to find Root Port for a PCI device") which added an extra check
> that would now be redundant.
> 
> Fixes: a99b646afa8a ("PCI: Disable PCIe Relaxed Ordering if unsupported")
> Fixes: c56d4450eb68 ("PCI: Turn off Request Attributes to avoid Chelsio T5 Completion erratum")
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

I *think* this should work for everybody, but I can't test it personally.

> ---
> This applies on top of and was tested on next-20170817.
> 
> Michael, it'd be great if you could test this one again to clarify
> whether or not the fix that's already in Linus' tree is still needed, or
> whether it's indeed obsoleted by this patch.
> 
>  drivers/pci/pci.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b05c587e335a..dd56c1c05614 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -514,7 +514,7 @@ EXPORT_SYMBOL(pci_find_resource);
>   */
>  struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>  {
> -	struct pci_dev *bridge, *highest_pcie_bridge = NULL;
> +	struct pci_dev *bridge, *highest_pcie_bridge = dev;
>  
>  	bridge = pci_upstream_bridge(dev);
>  	while (bridge && pci_is_pcie(bridge)) {
> @@ -522,11 +522,10 @@ struct pci_dev *pci_find_pcie_root_port(struct pci_dev *dev)
>  		bridge = pci_upstream_bridge(bridge);
>  	}
>  
> -	if (highest_pcie_bridge &&
> -	    pci_pcie_type(highest_pcie_bridge) == PCI_EXP_TYPE_ROOT_PORT)
> -		return highest_pcie_bridge;
> +	if (pci_pcie_type(highest_pcie_bridge) != PCI_EXP_TYPE_ROOT_PORT)
> +		return NULL;
>  
> -	return NULL;
> +	return highest_pcie_bridge;
>  }
>  EXPORT_SYMBOL(pci_find_pcie_root_port);
>  
> -- 
> 2.13.3
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* Re: [PATCH] vsock: only load vmci transport on VMware hypervisor by default
From: David Miller @ 2017-08-17 17:04 UTC (permalink / raw)
  To: decui
  Cc: mkubecek, jasowang, georgezhang, sthemmin, rolf.neugebauer,
	marcelo.cerri, asias, dan.carpenter, olaf, haiyangz, dave.scott,
	stefanha, apw, netdev, linux-kernel, gregkh, joe, devel, vkuznets,
	jhansen
In-Reply-To: <KL1P15301MB0008F70D4E5248AB7A296CE3BF830@KL1P15301MB0008.APCP153.PROD.OUTLOOK.COM>

From: Dexuan Cui <decui@microsoft.com>
Date: Thu, 17 Aug 2017 08:00:29 +0000

> @@ -73,6 +74,10 @@ struct vmci_transport_recv_pkt_info {
>  	struct vmci_transport_packet pkt;
>  };
>  
> +static bool skip_hypervisor_check;
> +module_param(skip_hypervisor_check, bool, 0444);
> +MODULE_PARM_DESC(hot_add, "If set, attempt to load on non-VMware platforms");
> +

I would avoid module parameters at all costs.

It is the worst possible interface for users of your software.

You really need to fundamentally solve the problems related to making
sure the proper modules for the VM actually present on the system get
loaded when necessary rather than adding hacks like this.

Unlike a proper solution, these hacks are ugly but have to stay around
forever once you put them in place.

^ permalink raw reply

* Re: [PATCH] liquidio: fix spelling mistake: "interuupt" -> "interrupt"
From: David Miller @ 2017-08-17 17:05 UTC (permalink / raw)
  To: colin.king
  Cc: derek.chickles, satananda.burla, felix.manlunas, raghu.vatsavayi,
	netdev, linux-kernel
In-Reply-To: <20170817081930.12986-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Thu, 17 Aug 2017 09:19:30 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> Trivial fix to spelling mistake in dev_info message
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH][net-next] net: hns3: ensure media_type is unitialized
From: David Miller @ 2017-08-17 17:06 UTC (permalink / raw)
  To: colin.king
  Cc: yisen.zhuang, salil.mehta, huangdaode, lipeng321, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20170817090107.14310-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Thu, 17 Aug 2017 10:01:07 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> Media type is only set if h->ae_algo->ops->get_media_type is called
> so there is a possibility that media_type is uninitialized when it is
> used a switch statement.  Fix this by initializing media_type to
> HNAE3_MEDIA_TYPE_UNKNOWN.
> 
> Detected by CoverityScan, CID#1452624("Uninitialized scalar variable")
> 
> Fixes: 496d03e960ae ("net: hns3: Add Ethtool support to HNS3 driver")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied.

^ permalink raw reply

* [iproute PATCH v2 0/2] Covscan: Fix potential file descriptor leaks
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This series collects patches from v1 which deal with potential file
descriptor leaks.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (2):
  ss: Don't leak fd in tcp_show_netlink_file()
  tc/em_ipset: Don't leak sockfd on error path

 misc/ss.c     | 32 ++++++++++++++++++++------------
 tc/em_ipset.c |  1 +
 2 files changed, 21 insertions(+), 12 deletions(-)

-- 
2.13.1

^ permalink raw reply

* [iproute PATCH v2 6/7] netem/maketable: Check return value of fstat()
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24302-1-phil@nwl.cc>

Otherwise info.st_size may contain garbage.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 netem/maketable.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/netem/maketable.c b/netem/maketable.c
index 6aff927be7040..ad660e7d457f0 100644
--- a/netem/maketable.c
+++ b/netem/maketable.c
@@ -24,8 +24,8 @@ readdoubles(FILE *fp, int *number)
 	int limit;
 	int n=0, i;
 
-	fstat(fileno(fp), &info);
-	if (info.st_size > 0) {
+	if (!fstat(fileno(fp), &info) &&
+	    info.st_size > 0) {
 		limit = 2*info.st_size/sizeof(double);	/* @@ approximate */
 	} else {
 		limit = 10000;
-- 
2.13.1

^ permalink raw reply related

* Re: [PATCH v2] bpf: Update sysctl documentation to list all supported architectures
From: David Miller @ 2017-08-17 17:09 UTC (permalink / raw)
  To: mpe; +Cc: ast, daniel, netdev, linux-kernel
In-Reply-To: <1502965839-23115-1-git-send-email-mpe@ellerman.id.au>

From: Michael Ellerman <mpe@ellerman.id.au>
Date: Thu, 17 Aug 2017 20:30:39 +1000

> The sysctl documentation states that the JIT is only available on
> x86_64, which is no longer correct.
> 
> Update the list, and break it out to indicate which architectures
> support the cBPF JIT (via HAVE_CBPF_JIT) or the eBPF JIT
> (HAVE_EBPF_JIT).
> 
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>

Applied, thanks Michael.

^ permalink raw reply

* [iproute PATCH v2 1/5] ifstat, nstat: Check fdopen() return value
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24545-1-phil@nwl.cc>

Prevent passing NULL FILE pointer to fgets() later.

Fix both tools in a single patch since the code changes are basically
identical.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ifstat.c | 16 +++++++++++-----
 misc/nstat.c  | 16 +++++++++++-----
 2 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/misc/ifstat.c b/misc/ifstat.c
index 1be21703bf14c..ac3eff6b870a9 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -992,12 +992,18 @@ int main(int argc, char *argv[])
 	    && verify_forging(fd) == 0) {
 		FILE *sfp = fdopen(fd, "r");
 
-		load_raw_table(sfp);
-		if (hist_db && source_mismatch) {
-			fprintf(stderr, "ifstat: history is stale, ignoring it.\n");
-			hist_db = NULL;
+		if (!sfp) {
+			fprintf(stderr, "ifstat: fdopen failed: %s\n",
+				strerror(errno));
+			close(fd);
+		} else  {
+			load_raw_table(sfp);
+			if (hist_db && source_mismatch) {
+				fprintf(stderr, "ifstat: history is stale, ignoring it.\n");
+				hist_db = NULL;
+			}
+			fclose(sfp);
 		}
-		fclose(sfp);
 	} else {
 		if (fd >= 0)
 			close(fd);
diff --git a/misc/nstat.c b/misc/nstat.c
index 1212b1f2c8128..a4dd405d43a93 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -706,12 +706,18 @@ int main(int argc, char *argv[])
 	    && verify_forging(fd) == 0) {
 		FILE *sfp = fdopen(fd, "r");
 
-		load_good_table(sfp);
-		if (hist_db && source_mismatch) {
-			fprintf(stderr, "nstat: history is stale, ignoring it.\n");
-			hist_db = NULL;
+		if (!sfp) {
+			fprintf(stderr, "nstat: fdopen failed: %s\n",
+				strerror(errno));
+			close(fd);
+		} else {
+			load_good_table(sfp);
+			if (hist_db && source_mismatch) {
+				fprintf(stderr, "nstat: history is stale, ignoring it.\n");
+				hist_db = NULL;
+			}
+			fclose(sfp);
 		}
-		fclose(sfp);
 	} else {
 		if (fd >= 0)
 			close(fd);
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 6/7] ss: Drop useless assignment
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24089-1-phil@nwl.cc>

After '*b = *a', 'b->next' already has the same value as 'a->next'.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 2debccce5260b..b2a7f069e294c 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -1440,7 +1440,6 @@ static int remember_he(struct aafilter *a, struct hostent *he)
 			if ((b = malloc(sizeof(*b))) == NULL)
 				return cnt;
 			*b = *a;
-			b->next = a->next;
 			a->next = b;
 		}
 		memcpy(b->addr.data, *ptr, len);
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 5/7] ss: Use C99 initializer in netlink_show_one()
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24302-1-phil@nwl.cc>

This has the additional benefit of initializing st.ino to zero which is
used later in is_sctp_assoc() function.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/misc/ss.c b/misc/ss.c
index b2a7f069e294c..d767b1103ea81 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3469,17 +3469,18 @@ static int netlink_show_one(struct filter *f,
 				int rq, int wq,
 				unsigned long long sk, unsigned long long cb)
 {
-	struct sockstat st;
+	struct sockstat st = {
+		.state		= SS_CLOSE,
+		.rq		= rq,
+		.wq		= wq,
+		.local.family	= AF_NETLINK,
+		.remote.family	= AF_NETLINK,
+	};
 
 	SPRINT_BUF(prot_buf) = {};
 	const char *prot_name;
 	char procname[64] = {};
 
-	st.state = SS_CLOSE;
-	st.rq	 = rq;
-	st.wq	 = wq;
-	st.local.family = st.remote.family = AF_NETLINK;
-
 	if (f->f) {
 		st.rport = -1;
 		st.lport = pid;
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 6/7] tc/m_xt: Fix for potential string buffer overflows
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170932.24659-1-phil@nwl.cc>

- Use strncpy() when writing to target->t->u.user.name and make sure the
  final byte remains untouched (xtables_calloc() set it to zero).
- 'tname' length sanitization was completely wrong: If it's length
  exceeded the 16 bytes available in 'k', passing a length value of 16
  to strncpy() would overwrite the previously NULL'ed 'k[15]'. Also, the
  sanitization has to happen if 'tname' is exactly 16 bytes long as
  well.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/m_xt.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tc/m_xt.c b/tc/m_xt.c
index ad52d239caf61..9218b14594403 100644
--- a/tc/m_xt.c
+++ b/tc/m_xt.c
@@ -95,7 +95,8 @@ build_st(struct xtables_target *target, struct xt_entry_target *t)
 	if (t == NULL) {
 		target->t = xtables_calloc(1, size);
 		target->t->u.target_size = size;
-		strcpy(target->t->u.user.name, target->name);
+		strncpy(target->t->u.user.name, target->name,
+			sizeof(target->t->u.user.name) - 1);
 		target->t->u.user.revision = target->revision;
 
 		if (target->init != NULL)
@@ -277,8 +278,8 @@ static int parse_ipt(struct action_util *a, int *argc_p,
 	}
 	fprintf(stdout, " index %d\n", index);
 
-	if (strlen(tname) > 16) {
-		size = 16;
+	if (strlen(tname) >= 16) {
+		size = 15;
 		k[15] = 0;
 	} else {
 		size = 1 + strlen(tname);
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 2/2] tc/em_ipset: Don't leak sockfd on error path
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24224-1-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/em_ipset.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tc/em_ipset.c b/tc/em_ipset.c
index fab975f5ea563..b59756515d239 100644
--- a/tc/em_ipset.c
+++ b/tc/em_ipset.c
@@ -84,6 +84,7 @@ static int get_version(unsigned int *version)
 	res = getsockopt(sockfd, SOL_IP, SO_IP_SET, &req_version, &size);
 	if (res != 0) {
 		perror("xt_set getsockopt");
+		close(sockfd);
 		return -1;
 	}
 
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 5/5] tipc/bearer: Prevent NULL pointer dereference
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24545-1-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tipc/bearer.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tipc/bearer.c b/tipc/bearer.c
index c3d4491f8f6ef..0598328ab1f1b 100644
--- a/tipc/bearer.c
+++ b/tipc/bearer.c
@@ -438,8 +438,8 @@ static int cmd_bearer_enable(struct nlmsghdr *nlh, const struct cmd *cmd,
 	if (err)
 		return err;
 
-	opt = get_opt(opts, "media");
-	if (strcmp(opt->val, "udp") == 0) {
+	if ((opt = get_opt(opts, "media")) &&
+	    strcmp(opt->val, "udp") == 0) {
 		err = nl_add_udp_enable_opts(nlh, opts, cmdl);
 		if (err)
 			return err;
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 4/5] ss: Fix potential memleak in unix_stats_print()
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24435-1-phil@nwl.cc>

Fixes: 2d0e538f3e1cd ("ss: Drop list traversal from unix_stats_print()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ss.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/misc/ss.c b/misc/ss.c
index 07eecfa7a36db..34c6da5443642 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -3169,8 +3169,10 @@ static int unix_show(struct filter *f)
 
 		if (name[0]) {
 			u->name = strdup(name);
-			if (!u->name)
+			if (!u->name) {
+				free(u);
 				break;
+			}
 		}
 
 		if (u->rport) {
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 3/7] iproute: Fix for missing 'Oifs:' display
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24089-1-phil@nwl.cc>

Covscan complained about dead code but after reading it, I assume the
author's intention was to prefix the interface list with 'Oifs: '.
Initializing first to 1 and setting it to 0 after above prefix was
printed should fix it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/iproute.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index cb695ad4141a7..89caac124f489 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -624,7 +624,7 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 	}
 	if (tb[RTA_MULTIPATH]) {
 		struct rtnexthop *nh = RTA_DATA(tb[RTA_MULTIPATH]);
-		int first = 0;
+		int first = 1;
 
 		len = RTA_PAYLOAD(tb[RTA_MULTIPATH]);
 
@@ -634,10 +634,12 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
 			if (nh->rtnh_len > len)
 				break;
 			if (r->rtm_flags&RTM_F_CLONED && r->rtm_type == RTN_MULTICAST) {
-				if (first)
+				if (first) {
 					fprintf(fp, "Oifs: ");
-				else
+					first = 0;
+				} else {
 					fprintf(fp, " ");
+				}
 			} else
 				fprintf(fp, "%s\tnexthop ", _SL_);
 			if (nh->rtnh_len > sizeof(*nh)) {
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 3/5] tc/q_netem: Don't dereference possibly NULL pointer
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24545-1-phil@nwl.cc>

Assuming 'opt' might be NULL, move the call to RTA_PAYLOAD to after the
check since it dereferences its parameter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/q_netem.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tc/q_netem.c b/tc/q_netem.c
index 0975ae111de97..7e3330512041a 100644
--- a/tc/q_netem.c
+++ b/tc/q_netem.c
@@ -538,7 +538,7 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	int *ecn = NULL;
 	struct tc_netem_qopt qopt;
 	const struct tc_netem_rate *rate = NULL;
-	int len = RTA_PAYLOAD(opt) - sizeof(qopt);
+	int len;
 	__u64 rate64 = 0;
 
 	SPRINT_BUF(b1);
@@ -546,6 +546,8 @@ static int netem_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
 	if (opt == NULL)
 		return 0;
 
+	len = RTA_PAYLOAD(opt) - sizeof(qopt);
+
 	if (len < 0) {
 		fprintf(stderr, "options size error\n");
 		return -1;
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 1/7] devlink: No need for this self-assignment
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24089-1-phil@nwl.cc>

dl_argv_handle_both() will either assign to handle_bit or error out in
which case the variable is not used by the caller.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 devlink/devlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/devlink/devlink.c b/devlink/devlink.c
index f9bc16c350c40..bf43e2cd5e709 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -779,7 +779,7 @@ static int dl_argv_parse(struct dl *dl, uint32_t o_required,
 	int err;
 
 	if (o_required & DL_OPT_HANDLE && o_required & DL_OPT_HANDLEP) {
-		uint32_t handle_bit = handle_bit;
+		uint32_t handle_bit;
 
 		err = dl_argv_handle_both(dl, &opts->bus_name, &opts->dev_name,
 					  &opts->port_index, &handle_bit);
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH v2 2/5] ifstat: Fix memleak in error case
From: Phil Sutter @ 2017-08-17 17:09 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170817170931.24435-1-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/ifstat.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/misc/ifstat.c b/misc/ifstat.c
index a853ee6d7e3b3..8fa354265a9a1 100644
--- a/misc/ifstat.c
+++ b/misc/ifstat.c
@@ -143,8 +143,10 @@ static int get_nlmsg_extended(const struct sockaddr_nl *who,
 		struct rtattr *attr;
 
 		attr = parse_rtattr_one_nested(sub_type, tb[filter_type]);
-		if (attr == NULL)
+		if (attr == NULL) {
+			free(n);
 			return 0;
+		}
 		memcpy(&n->val, RTA_DATA(attr), sizeof(n->val));
 	}
 	memset(&n->rate, 0, sizeof(n->rate));
-- 
2.13.1

^ permalink raw reply related


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