Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] vhost/scsi: use vmalloc for order-10 allocation
From: Sergei Shtylyov @ 2013-09-17 18:14 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: kvm, netdev, linux-kernel, virtualization, Dan Aloni
In-Reply-To: <1379401998-5131-1-git-send-email-mst@redhat.com>

Hello.

On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:

> As vhost scsi device struct is large, if the device is
> created on a busy system, kzalloc() might fail, so this patch does a
> fallback to vzalloc().

> As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> to kzalloc() flags to do this fallback only when really needed.

> Reported-by: Dan Aloni <alonid@stratoscale.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---

> I put this on my vhost fixes branch, intend to merge for 3.12.
> Dan, could you please confirm this works for you?

>   drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
>   1 file changed, 27 insertions(+), 14 deletions(-)

> diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> index 4b79a1f..2c30bb0 100644
> --- a/drivers/vhost/scsi.c
> +++ b/drivers/vhost/scsi.c
> @@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
>   	return 0;
>   }
>
> +static void vhost_scsi_free(struct vhost_scsi *vs)
> +{
> +        if (is_vmalloc_addr(vs))
> +                vfree(vs);
> +        else
> +                kfree(vs);

    Indent with the tabs ISO spaces, please.

> +}
> +
>   static int vhost_scsi_open(struct inode *inode, struct file *f)
>   {
>   	struct vhost_scsi *vs;
>   	struct vhost_virtqueue **vqs;
> -	int r, i;
> +	int r = -ENOMEM, i;
>
> -	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> -	if (!vs)
> -		return -ENOMEM;
> +        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);

    Indent here with a tab, please.

> +	if (!vs) {
> +		vs = vzalloc(sizeof(*vs));
> +		if (!vs)
> +			goto err_vs;
> +	}

WBR, Sergei

^ permalink raw reply

* Re: [PATCH v2.39 7/7] datapath: Add basic MPLS support to kernel
From: Pravin Shelar @ 2013-09-17 18:38 UTC (permalink / raw)
  To: Simon Horman
  Cc: dev@openvswitch.org, netdev, Ravi K, Isaku Yamahata, Jesse Gross,
	Joe Stringer
In-Reply-To: <1378711207-29890-8-git-send-email-horms@verge.net.au>

On Mon, Sep 9, 2013 at 12:20 AM, Simon Horman <horms@verge.net.au> wrote:
> Allow datapath to recognize and extract MPLS labels into flow keys
> and execute actions which push, pop, and set labels on packets.
>
> Based heavily on work by Leo Alterman, Ravi K, Isaku Yamahata and Joe Stringer.
>
> Cc: Ravi K <rkerur@gmail.com>
> Cc: Leo Alterman <lalterman@nicira.com>
> Cc: Isaku Yamahata <yamahata@valinux.co.jp>
> Cc: Joe Stringer <joe@wand.net.nz>
> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
....
> diff --git a/datapath/datapath.h b/datapath/datapath.h
> index 5d50dd4..babae3b 100644
> --- a/datapath/datapath.h
> +++ b/datapath/datapath.h
> @@ -36,6 +36,10 @@
>
>  #define SAMPLE_ACTION_DEPTH 3
>
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,11,0)
> +#define HAVE_INNER_PROTOCOL
> +#endif
> +
>  /**
>   * struct dp_stats_percpu - per-cpu packet processing statistics for a given
>   * datapath.
> @@ -93,11 +97,16 @@ struct datapath {
>   * @pkt_key: The flow information extracted from the packet.  Must be nonnull.
>   * @tun_key: Key for the tunnel that encapsulated this packet. NULL if the
>   * packet is not being tunneled.
> + * @inner_protocol: Provides a substitute for the skb->inner_protocol field on
> + * kernels before 3.11.
>   */
>  struct ovs_skb_cb {
>         struct sw_flow          *flow;
>         struct sw_flow_key      *pkt_key;
>         struct ovs_key_ipv4_tunnel  *tun_key;
> +#ifndef HAVE_INNER_PROTOCOL
> +       __be16                  inner_protocol;
> +#endif
>  };
>  #define OVS_CB(skb) ((struct ovs_skb_cb *)(skb)->cb)
>
Can you move this to compat struct ovs_gso_cb {}

....
>
> +struct sk_buff *rpl___skb_gso_segment(struct sk_buff *skb,
> +                                     netdev_features_t features,
> +                                     bool tx_path)
> +{
> +       struct sk_buff *skb_gso;
> +       __be16 type = skb->protocol;
> +
> +       skb->protocol = skb_network_protocol(skb);
> +
> +       /* this hack needed to get regular skb_gso_segment() */
> +#ifdef HAVE___SKB_GSO_SEGMENT
> +#undef __skb_gso_segment
> +       skb_gso = __skb_gso_segment(skb, features, tx_path);
> +#else
> +#undef skb_gso_segment
> +       skb_gso = skb_gso_segment(skb, features);
> +#endif
> +
> +       if (!skb_gso || IS_ERR(skb_gso))
> +           return skb_gso;
> +
> +       skb = skb_gso;
> +       while (skb) {
> +               skb->protocol = type;
> +               skb = skb->next;
> +       }
> +

Protocol set is required if there is MPLS header, which is rare case.
So I think we can skip this loop if there is no mpls.

> +       return skb_gso;
> +}
> +
> +struct sk_buff *rpl_skb_gso_segment(struct sk_buff *skb,
.....
> +
> +       if (vlan_tx_tag_present(skb) && !dev_supports_vlan_tx(skb->dev))
> +               vlan = true;
> +
> +       if (vlan || mpls) {
> +               netdev_features_t features;
>
>                 features = netif_skb_features(skb);
>
> @@ -296,6 +309,20 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                         features &= ~(NETIF_F_TSO | NETIF_F_TSO6 |
>                                       NETIF_F_UFO | NETIF_F_FSO);
>
> +               /* As of v3.11 the kernel provides an mpls_features field in
> +                * struct net_device which allows devices to advertise which
> +                * features its supports for MPLS. This value defaults to
> +                * NETIF_F_SG and as of v3.11.
> +                *
> +                * This compatibility code is intended for kernels older
> +                * than v3.11 that do not support MPLS GSO and thus do not
> +                * provide mpls_features. Thus this code uses NETIF_F_SG
> +                * directly in place of mpls_features.
> +                */
> +
> +               if (mpls)
> +                       features &= NETIF_F_SG;
> +
>                 if (netif_needs_gso(skb, features)) {
>                         struct sk_buff *nskb;
>
> @@ -319,10 +346,12 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                                 nskb = skb->next;
>                                 skb->next = NULL;
>
> -                               skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> +                               if (vlan)
> +                                       skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
>                                 if (likely(skb)) {
>                                         len += skb->len;
> -                                       vlan_set_tci(skb, 0);
> +                                       if (vlan)
> +                                               vlan_set_tci(skb, 0);
>                                         dev_queue_xmit(skb);
>                                 }
>
> @@ -333,10 +362,12 @@ static int netdev_send(struct vport *vport, struct sk_buff *skb)
>                 }
>
>  tag:
> -               skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> -               if (unlikely(!skb))
> -                       return 0;
> -               vlan_set_tci(skb, 0);
> +               if (vlan) {
> +                       skb = __vlan_put_tag(skb, skb->vlan_proto, vlan_tx_tag_get(skb));
> +                       if (unlikely(!skb))
> +                               return 0;
> +                       vlan_set_tci(skb, 0);
> +               }
>         }
>
I think we can simplify code by pushing vlan and then segmenting skb,
the way we do it for MPLS.

^ permalink raw reply

* [PATCH] vxlan: Avoid creating fdb entry with NULL destination
From: Sridhar Samudrala @ 2013-09-17 19:12 UTC (permalink / raw)
  To: davem; +Cc: netdev, stephen, mike.rapoport

Commit afbd8bae9c798c5cdbe4439d3a50536b5438247c 
   vxlan: add implicit fdb entry for default destination
creates an implicit fdb entry for default destination. This results 
in an invalid fdb entry if default destination is not specified.
For ex: 
  ip link add vxlan1 type vxlan id 100
creates the following fdb entry
  00:00:00:00:00:00 dev vxlan1 dst 0.0.0.0 self permanent

This patch fixes this issue by creating an fdb entry only if a
valid default destination is specified.

Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
---

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bf64b41..ac25c2d 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2492,15 +2492,19 @@ static int vxlan_newlink(struct net *net, struct net_device *dev,
 
 	SET_ETHTOOL_OPS(dev, &vxlan_ethtool_ops);
 
-	/* create an fdb entry for default destination */
-	err = vxlan_fdb_create(vxlan, all_zeros_mac,
-			       &vxlan->default_dst.remote_ip,
-			       NUD_REACHABLE|NUD_PERMANENT,
-			       NLM_F_EXCL|NLM_F_CREATE,
-			       vxlan->dst_port, vxlan->default_dst.remote_vni,
-			       vxlan->default_dst.remote_ifindex, NTF_SELF);
-	if (err)
-		return err;
+	/* create an fdb entry for a valid default destination */
+	if (!vxlan_addr_any(&vxlan->default_dst.remote_ip)) {
+		err = vxlan_fdb_create(vxlan, all_zeros_mac,
+				       &vxlan->default_dst.remote_ip,
+				       NUD_REACHABLE|NUD_PERMANENT,
+				       NLM_F_EXCL|NLM_F_CREATE,
+				       vxlan->dst_port,
+				       vxlan->default_dst.remote_vni,
+				       vxlan->default_dst.remote_ifindex,
+				       NTF_SELF);
+		if (err)
+			return err;
+	}
 
 	err = register_netdevice(dev);
 	if (err) {

^ permalink raw reply related

* [PATCH 1/2] Re: net, vxlan Fix compile warning
From: Prarit Bhargava @ 2013-09-17 19:12 UTC (permalink / raw)
  To: netdev; +Cc: Prarit Bhargava, jpirko, davem, stephen
In-Reply-To: <20130916.212358.1356743853860170770.davem@davemloft.net>

Fix a unintialized variable warning.

drivers/net/vxlan.c: In function ‘vxlan_sock_add’:
drivers/net/vxlan.c:2240:11: error: ‘sock’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  vs->sock = sock;
           ^
drivers/net/vxlan.c:2217:17: note: ‘sock’ was declared here
  struct socket *sock;
                 ^
[v2]: davem suggested resolving this by making create_v{4,6}_sock() return an
err pointer.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: jpirko@redhat.com
Cc: davem@davemloft.net
Cc: stephen@networkplumber.org
---
 drivers/net/vxlan.c |   26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index bf64b41..6ec6aa4 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -2184,7 +2184,7 @@ static void vxlan_del_work(struct work_struct *work)
  * could be used for both IPv4 and IPv6 communications, but
  * users may set bindv6only=1.
  */
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
 {
 	struct sock *sk;
 	struct socket *sock;
@@ -2197,7 +2197,7 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
 	rc = sock_create_kern(AF_INET6, SOCK_DGRAM, IPPROTO_UDP, &sock);
 	if (rc < 0) {
 		pr_debug("UDPv6 socket create failed\n");
-		return rc;
+		return ERR_PTR(rc);
 	}
 
 	/* Put in proper namespace */
@@ -2212,28 +2212,27 @@ static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
 		pr_debug("bind for UDPv6 socket %pI6:%u (%d)\n",
 			 &vxlan_addr.sin6_addr, ntohs(vxlan_addr.sin6_port), rc);
 		sk_release_kernel(sk);
-		return rc;
+		return ERR_PTR(rc);
 	}
 	/* At this point, IPv6 module should have been loaded in
 	 * sock_create_kern().
 	 */
 	BUG_ON(!ipv6_stub);
 
-	*psock = sock;
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
-	return 0;
+	return sock;
 }
 
 #else
 
-static int create_v6_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v6_sock(struct net *net, __be16 port)
 {
-		return -EPFNOSUPPORT;
+		return ERR_PTR(-EPFNOSUPPORT);
 }
 #endif
 
-static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
+static struct socket *create_v4_sock(struct net *net, __be16 port)
 {
 	struct sock *sk;
 	struct socket *sock;
@@ -2248,7 +2247,7 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
 	rc = sock_create_kern(AF_INET, SOCK_DGRAM, IPPROTO_UDP, &sock);
 	if (rc < 0) {
 		pr_debug("UDP socket create failed\n");
-		return rc;
+		return ERR_PTR(rc);
 	}
 
 	/* Put in proper namespace */
@@ -2261,13 +2260,12 @@ static int create_v4_sock(struct net *net, __be16 port, struct socket **psock)
 		pr_debug("bind for UDP socket %pI4:%u (%d)\n",
 			 &vxlan_addr.sin_addr, ntohs(vxlan_addr.sin_port), rc);
 		sk_release_kernel(sk);
-		return rc;
+		return ERR_PTR(rc);
 	}
 
-	*psock = sock;
 	/* Disable multicast loopback */
 	inet_sk(sk)->mc_loop = 0;
-	return 0;
+	return sock;
 }
 
 /* Create new listen socket if needed */
@@ -2291,9 +2289,9 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
 	INIT_WORK(&vs->del_work, vxlan_del_work);
 
 	if (ipv6)
-		rc = create_v6_sock(net, port, &sock);
+		sock = create_v6_sock(net, port);
 	else
-		rc = create_v4_sock(net, port, &sock);
+		sock = create_v4_sock(net, port);
 	if (rc < 0) {
 		kfree(vs);
 		return ERR_PTR(rc);
-- 
1.7.9.3

^ permalink raw reply related

* [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: Prarit Bhargava @ 2013-09-17 19:13 UTC (permalink / raw)
  To: netdev; +Cc: Prarit Bhargava, dledford, amirv, davem, ogerlitz
In-Reply-To: <20130916.212559.252825159014181091.davem@davemloft.net>

Fix unitialized variable warnings.

drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  atomic_dec(&cq->mtt->ref_count);
                ^
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  atomic_dec(&srq->mtt->ref_count);

[v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
pointer instead of setting 'cq' by reference.  I also did the same for
srq.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
Cc: dledford@redhat.com
Cc: amirv@mellanox.com
Cc: davem@davemloft.net
Cc: ogerlitz@mellanox.com
---
 .../net/ethernet/mellanox/mlx4/resource_tracker.c  |   46 ++++++++++----------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
index dd68763..343206b 100644
--- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
+++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
@@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
 	return err;
 }
 
-static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
-				enum res_cq_states state, struct res_cq **cq)
+static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
+						  int slave, int cqn,
+						  enum res_cq_states state)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
 	struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
 			r->com.from_state = r->com.state;
 			r->com.to_state = state;
 			r->com.state = RES_CQ_BUSY;
-			if (cq)
-				*cq = r;
+		} else {
+			r = ERR_PTR(err);
 		}
 	}
 
 	spin_unlock_irq(mlx4_tlock(dev));
 
-	return err;
+	return r;
 }
 
-static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
-				 enum res_cq_states state, struct res_srq **srq)
+static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
+					     int index,
+					     enum res_cq_states state)
 {
 	struct mlx4_priv *priv = mlx4_priv(dev);
 	struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
@@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
 			r->com.from_state = r->com.state;
 			r->com.to_state = state;
 			r->com.state = RES_SRQ_BUSY;
-			if (srq)
-				*srq = r;
+		} else {
+			r = ERR_PTR(err);
 		}
 	}
 
 	spin_unlock_irq(mlx4_tlock(dev));
 
-	return err;
+	return r;
 }
 
 static void res_abort_move(struct mlx4_dev *dev, int slave,
@@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
 	struct res_cq *cq;
 	struct res_mtt *mtt;
 
-	err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
-	if (err)
-		return err;
+	cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
+	if (IS_ERR(cq))
+		return PTR_ERR(cq);
 	err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
 	if (err)
 		goto out_move;
@@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
 	int cqn = vhcr->in_modifier;
 	struct res_cq *cq;
 
-	err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
-	if (err)
-		return err;
+	cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
+	if (IS_ERR(cq))
+		return PTR_ERR(cq);
 	err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
 	if (err)
 		goto out_move;
@@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
 	if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
 		return -EINVAL;
 
-	err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
-	if (err)
-		return err;
+	srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+	if (IS_ERR(srq))
+		return PTR_ERR(srq);
 	err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
 	if (err)
 		goto ex_abort;
@@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
 	int srqn = vhcr->in_modifier;
 	struct res_srq *srq;
 
-	err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
-	if (err)
-		return err;
+	srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
+	if (IS_ERR(srq))
+		return PTR_ERR(srq);
 	err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
 	if (err)
 		goto ex_abort;
-- 
1.7.9.3

^ permalink raw reply related

* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: Or Gerlitz @ 2013-09-17 19:25 UTC (permalink / raw)
  To: Prarit Bhargava, Jack Morgenstein
  Cc: netdev@vger.kernel.org, Doug Ledford, Amir Vadai, David Miller,
	Or Gerlitz
In-Reply-To: <1379445223-28550-1-git-send-email-prarit@redhat.com>

On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
> Fix unitialized variable warnings.
>
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   atomic_dec(&cq->mtt->ref_count);
>                 ^
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   atomic_dec(&srq->mtt->ref_count);
>
> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
> pointer instead of setting 'cq' by reference.  I also did the same for srq.

Pravit, as I wrote you earlier on this thread, Jack from our team
maintains this piece (SRIOV resource tracker) of the mlx4 core driver
code, so I am adding him. Dave, as many of us here he might be OOO for
the coming ten days for holiday vacation, so would ask you to please
wait patiently for his ack/nak...

Or.

> Signed-off-by: Prarit Bhargava <prarit@redhat.com>
> Cc: dledford@redhat.com
> Cc: amirv@mellanox.com
> Cc: davem@davemloft.net
> Cc: ogerlitz@mellanox.com
> ---
>  .../net/ethernet/mellanox/mlx4/resource_tracker.c  |   46 ++++++++++----------
>  1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> index dd68763..343206b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/resource_tracker.c
> @@ -1073,8 +1073,9 @@ static int eq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
>         return err;
>  }
>
> -static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
> -                               enum res_cq_states state, struct res_cq **cq)
> +static struct res_cq *cq_res_start_move_to(struct mlx4_dev *dev,
> +                                                 int slave, int cqn,
> +                                                 enum res_cq_states state)
>  {
>         struct mlx4_priv *priv = mlx4_priv(dev);
>         struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1117,18 +1118,19 @@ static int cq_res_start_move_to(struct mlx4_dev *dev, int slave, int cqn,
>                         r->com.from_state = r->com.state;
>                         r->com.to_state = state;
>                         r->com.state = RES_CQ_BUSY;
> -                       if (cq)
> -                               *cq = r;
> +               } else {
> +                       r = ERR_PTR(err);
>                 }
>         }
>
>         spin_unlock_irq(mlx4_tlock(dev));
>
> -       return err;
> +       return r;
>  }
>
> -static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
> -                                enum res_cq_states state, struct res_srq **srq)
> +static struct res_srq *srq_res_start_move_to(struct mlx4_dev *dev, int slave,
> +                                            int index,
> +                                            enum res_cq_states state)
>  {
>         struct mlx4_priv *priv = mlx4_priv(dev);
>         struct mlx4_resource_tracker *tracker = &priv->mfunc.master.res_tracker;
> @@ -1167,14 +1169,14 @@ static int srq_res_start_move_to(struct mlx4_dev *dev, int slave, int index,
>                         r->com.from_state = r->com.state;
>                         r->com.to_state = state;
>                         r->com.state = RES_SRQ_BUSY;
> -                       if (srq)
> -                               *srq = r;
> +               } else {
> +                       r = ERR_PTR(err);
>                 }
>         }
>
>         spin_unlock_irq(mlx4_tlock(dev));
>
> -       return err;
> +       return r;
>  }
>
>  static void res_abort_move(struct mlx4_dev *dev, int slave,
> @@ -2530,9 +2532,9 @@ int mlx4_SW2HW_CQ_wrapper(struct mlx4_dev *dev, int slave,
>         struct res_cq *cq;
>         struct res_mtt *mtt;
>
> -       err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW, &cq);
> -       if (err)
> -               return err;
> +       cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_HW);
> +       if (IS_ERR(cq))
> +               return PTR_ERR(cq);
>         err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
>         if (err)
>                 goto out_move;
> @@ -2565,9 +2567,9 @@ int mlx4_HW2SW_CQ_wrapper(struct mlx4_dev *dev, int slave,
>         int cqn = vhcr->in_modifier;
>         struct res_cq *cq;
>
> -       err = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED, &cq);
> -       if (err)
> -               return err;
> +       cq = cq_res_start_move_to(dev, slave, cqn, RES_CQ_ALLOCATED);
> +       if (IS_ERR(cq))
> +               return PTR_ERR(cq);
>         err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
>         if (err)
>                 goto out_move;
> @@ -2709,9 +2711,9 @@ int mlx4_SW2HW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
>         if (srqn != (be32_to_cpu(srqc->state_logsize_srqn) & 0xffffff))
>                 return -EINVAL;
>
> -       err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_HW, &srq);
> -       if (err)
> -               return err;
> +       srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> +       if (IS_ERR(srq))
> +               return PTR_ERR(srq);
>         err = get_res(dev, slave, mtt_base, RES_MTT, &mtt);
>         if (err)
>                 goto ex_abort;
> @@ -2748,9 +2750,9 @@ int mlx4_HW2SW_SRQ_wrapper(struct mlx4_dev *dev, int slave,
>         int srqn = vhcr->in_modifier;
>         struct res_srq *srq;
>
> -       err = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED, &srq);
> -       if (err)
> -               return err;
> +       srq = srq_res_start_move_to(dev, slave, srqn, RES_SRQ_ALLOCATED);
> +       if (IS_ERR(srq))
> +               return PTR_ERR(srq);
>         err = mlx4_DMA_wrapper(dev, slave, vhcr, inbox, outbox, cmd);
>         if (err)
>                 goto ex_abort;
> --
> 1.7.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/2] Re: net, mellanox mlx4 Fix compile warnings
From: David Miller @ 2013-09-17 19:28 UTC (permalink / raw)
  To: or.gerlitz; +Cc: prarit, jackm, netdev, dledford, amirv, ogerlitz
In-Reply-To: <CAJZOPZJ5kBW3_NQiw3S-DvDkYonzrW-EgxXdAqRcS1KGUrQu=w@mail.gmail.com>

From: Or Gerlitz <or.gerlitz@gmail.com>
Date: Tue, 17 Sep 2013 22:25:57 +0300

> On Tue, Sep 17, 2013 at 10:13 PM, Prarit Bhargava <prarit@redhat.com> wrote:
>> Fix unitialized variable warnings.
>>
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_CQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2551:16: error: ‘cq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   atomic_dec(&cq->mtt->ref_count);
>>                 ^
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c: In function ‘mlx4_HW2SW_SRQ_wrapper’:
>> drivers/net/ethernet/mellanox/mlx4/resource_tracker.c:2734:17: error: ‘srq’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>>   atomic_dec(&srq->mtt->ref_count);
>>
>> [v2]: davem suggested making cq_res_start_move_to() return 'cq' as an error
>> pointer instead of setting 'cq' by reference.  I also did the same for srq.
> 
> Pravit, as I wrote you earlier on this thread, Jack from our team
> maintains this piece (SRIOV resource tracker) of the mlx4 core driver
> code, so I am adding him. Dave, as many of us here he might be OOO for
> the coming ten days for holiday vacation, so would ask you to please
> wait patiently for his ack/nak...

I already stated that I wanted this issue fixed differently.

Specifically I said that these functions should return error pointers,
instead of trying to return an integer error whilst setting the cq
pointer by reference.

This is a method which is idiomatic and common across the kernel, and
is the preferred way to handle this kind of situation.

^ permalink raw reply

* Re: [PATCH] sh_eth: call phy_scan_fixups() after PHY reset
From: David Miller @ 2013-09-17 19:44 UTC (permalink / raw)
  To: sergei.shtylyov
  Cc: netdev, nobuhiro.iwamatsu.yj, linux-sh, laurent.pinchart+renesas
In-Reply-To: <201309140410.38396.sergei.shtylyov@cogentembedded.com>

From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sat, 14 Sep 2013 04:10:37 +0400

> Sometimes the PHY reset that sh_eth_phy_start() does effects the PHY registers
> registers values of which are vital for the correct functioning of the driver.
> Unfortunately, the existing PHY platform fixup mechanism doesn't help  here as
> it only hooks PHY resets done by ioctl() calls. Calling phy_scan_fixups() from
> the driver helps here. With a proper platform fixup, this fixes NFS timeouts on
> the SH-Mobile Lager board. 
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

The PHY layer is designed to naturally already take care of this kind of
thing.  I think that part of the problem is that you're fighting the
natural control flow the PHY layer provides.

When the phy_connect() is performed, what we end up doing is calling
phy_attach_direct() which invokes the ->probe() method of the driver
and then afterwards we do phy_init_hw() which takes care of doing
the fixup calls.

So if you really need to do a BMCR reset then run the fixups I'd like
you to look into making that happen within the provided control
flow rather than with an exceptional explicit call to run the fixups.

I'm willing to be convinced that this is a better or necessary approach
but you'll need to explain it to me.

Thanks.

^ permalink raw reply

* Re: [PATCH] vhost/scsi: use vmalloc for order-10 allocation
From: Michael S. Tsirkin @ 2013-09-17 19:51 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: kvm, netdev, linux-kernel, virtualization, Dan Aloni
In-Reply-To: <52389C0D.7070307@cogentembedded.com>

On Tue, Sep 17, 2013 at 10:14:37PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 09/17/2013 11:21 AM, Michael S. Tsirkin wrote:
> 
> >As vhost scsi device struct is large, if the device is
> >created on a busy system, kzalloc() might fail, so this patch does a
> >fallback to vzalloc().
> 
> >As vmalloc() adds overhead on data-path, add __GFP_REPEAT
> >to kzalloc() flags to do this fallback only when really needed.
> 
> >Reported-by: Dan Aloni <alonid@stratoscale.com>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >---
> 
> >I put this on my vhost fixes branch, intend to merge for 3.12.
> >Dan, could you please confirm this works for you?
> 
> >  drivers/vhost/scsi.c | 41 +++++++++++++++++++++++++++--------------
> >  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> >diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
> >index 4b79a1f..2c30bb0 100644
> >--- a/drivers/vhost/scsi.c
> >+++ b/drivers/vhost/scsi.c
> >@@ -1373,21 +1373,30 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
> >  	return 0;
> >  }
> >
> >+static void vhost_scsi_free(struct vhost_scsi *vs)
> >+{
> >+        if (is_vmalloc_addr(vs))
> >+                vfree(vs);
> >+        else
> >+                kfree(vs);
> 
>    Indent with the tabs ISO spaces, please.
> 
> >+}
> >+
> >  static int vhost_scsi_open(struct inode *inode, struct file *f)
> >  {
> >  	struct vhost_scsi *vs;
> >  	struct vhost_virtqueue **vqs;
> >-	int r, i;
> >+	int r = -ENOMEM, i;
> >
> >-	vs = kzalloc(sizeof(*vs), GFP_KERNEL);
> >-	if (!vs)
> >-		return -ENOMEM;
> >+        vs = kzalloc(sizeof(*vs), GFP_KERNEL | __GFP_NOWARN | __GFP_REPEAT);
> 
>    Indent here with a tab, please.
> 
> >+	if (!vs) {
> >+		vs = vzalloc(sizeof(*vs));
> >+		if (!vs)
> >+			goto err_vs;
> >+	}
> 
> WBR, Sergei

Thanks, I'll fix this up.

^ permalink raw reply

* ip6_finish_output2 change broke netfilter xt_TEE target
From: Phil Oester @ 2013-09-17 19:54 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev, netfilter-devel

The change made in commit 6fd6ce20 (ipv6: Do not depend on rt->n in
ip6_finish_output2) broke the xt_TEE target for IPv6 packets.  Instead
of using the nexthop provided in the --gateway option, ip6_finish_output2
is now performing neighbor solicitation for the original daddr in the
copied skb.

Similar breakage occurred in IPv4, and was fixed (in 2ad5b9e4) by using
the flag FLOWI_FLAG_KNOWN_NH.  I can find no easy way to make use of that
flag here.  Reverting 6fd6ce20 makes TEE work again, but I am not clear
on what problem that commit was attempting to solve.  Yoshifuji?

Phil


^ permalink raw reply

* Re: [PATCH 1/2] Re: net, vxlan Fix compile warning
From: Ben Hutchings @ 2013-09-17 20:26 UTC (permalink / raw)
  To: Prarit Bhargava; +Cc: netdev, jpirko, davem, stephen
In-Reply-To: <1379445167-28488-1-git-send-email-prarit@redhat.com>

You don't seem to have completed this change:

On Tue, 2013-09-17 at 15:12 -0400, Prarit Bhargava wrote:
[...]
> @@ -2291,9 +2289,9 @@ static struct vxlan_sock *vxlan_socket_create(struct net *net, __be16 port,
>  	INIT_WORK(&vs->del_work, vxlan_del_work);
>  
>  	if (ipv6)
> -		rc = create_v6_sock(net, port, &sock);
> +		sock = create_v6_sock(net, port);
>  	else
> -		rc = create_v4_sock(net, port, &sock);
> +		sock = create_v4_sock(net, port);
>  	if (rc < 0) {

	if (IS_ERR(sock)) {

>  		kfree(vs);
>  		return ERR_PTR(rc);

		return ERR_CAST(sock);

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply

* [PATCH] p54usb: fix leak at failure path in p54u_load_firmware()
From: Alexey Khoroshilov @ 2013-09-17 20:57 UTC (permalink / raw)
  To: Christian Lamparter
  Cc: Alexey Khoroshilov, John W. Linville, linux-wireless, netdev,
	linux-kernel, ldv-project

If request_firmware_nowait() fails in p54u_load_firmware(),
p54u_load_firmware_cb is not called and no one decrements usb_dev refcnt.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/net/wireless/p54/p54usb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index b9deef6..7fa81d1 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -979,6 +979,7 @@ static int p54u_load_firmware(struct ieee80211_hw *dev,
 	if (err) {
 		dev_err(&priv->udev->dev, "(p54usb) cannot load firmware %s "
 					  "(%d)!\n", p54u_fwlist[i].fw, err);
+		usb_put_dev(udev);
 	}
 
 	return err;
-- 
1.8.1.2

^ permalink raw reply related

* ath9k: remove self-assignment in ath9k_eeprom_request
From: Dave Jones @ 2013-09-17 21:35 UTC (permalink / raw)
  To: netdev; +Cc: mcgrof, linville

Assigning a variable to itself is pointless, moreso when it's overwritten immediately.

Introduced in ab5c4f71d8c7add173a2d32e5beefdaaf1b7cbbc
 ("ath9k: allow to load EEPROM content via firmware API")

Signed-off-by: Dave Jones <davej@fedoraproject.org>

diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 9a1f349..0cb3f2d 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -572,7 +572,7 @@ static void ath9k_eeprom_request_cb(const struct firmware *eeprom_blob,
 static int ath9k_eeprom_request(struct ath_softc *sc, const char *name)
 {
 	struct ath9k_eeprom_ctx ec;
-	struct ath_hw *ah = ah = sc->sc_ah;
+	struct ath_hw *ah = sc->sc_ah;
 	int err;
 
 	/* try to load the EEPROM content asynchronously */

^ permalink raw reply related

* [PATCH 1/7] netfilter: nf_conntrack: use RCU safe kfree for conntrack extensions
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Michal Kubeček <mkubecek@suse.cz>

Commit 68b80f11 (netfilter: nf_nat: fix RCU races) introduced
RCU protection for freeing extension data when reallocation
moves them to a new location. We need the same protection when
freeing them in nf_ct_ext_free() in order to prevent a
use-after-free by other threads referencing a NAT extension data
via bysource list.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/net/netfilter/nf_conntrack_extend.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/netfilter/nf_conntrack_extend.h b/include/net/netfilter/nf_conntrack_extend.h
index ff95434..88a1d40 100644
--- a/include/net/netfilter/nf_conntrack_extend.h
+++ b/include/net/netfilter/nf_conntrack_extend.h
@@ -86,7 +86,7 @@ static inline void nf_ct_ext_destroy(struct nf_conn *ct)
 static inline void nf_ct_ext_free(struct nf_conn *ct)
 {
 	if (ct->ext)
-		kfree(ct->ext);
+		kfree_rcu(ct->ext, rcu);
 }
 
 /* Add this type, returns pointer to data or NULL. */
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 3/7] netfilter: ipset: Skip really non-first fragments for IPv6 when getting port/protocol
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_getport.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_getport.c b/net/netfilter/ipset/ip_set_getport.c
index 6fdf88a..dac156f 100644
--- a/net/netfilter/ipset/ip_set_getport.c
+++ b/net/netfilter/ipset/ip_set_getport.c
@@ -116,12 +116,12 @@ ip_set_get_ip6_port(const struct sk_buff *skb, bool src,
 {
 	int protoff;
 	u8 nexthdr;
-	__be16 frag_off;
+	__be16 frag_off = 0;
 
 	nexthdr = ipv6_hdr(skb)->nexthdr;
 	protoff = ipv6_skip_exthdr(skb, sizeof(struct ipv6hdr), &nexthdr,
 				   &frag_off);
-	if (protoff < 0)
+	if (protoff < 0 || (frag_off & htons(~0x7)) != 0)
 		return false;
 
 	return get_port(skb, nexthdr, protoff, src, port, proto);
-- 
1.7.10.4


^ permalink raw reply related

* [PATCH 7/7] netfilter: nfnetlink_queue: use network skb for sequence adjustment
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Gao feng <gaofeng@cn.fujitsu.com>

Instead of the netlink skb.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_queue_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/nfnetlink_queue_core.c b/net/netfilter/nfnetlink_queue_core.c
index 95a98c8..ae2e5c1 100644
--- a/net/netfilter/nfnetlink_queue_core.c
+++ b/net/netfilter/nfnetlink_queue_core.c
@@ -1009,7 +1009,7 @@ nfqnl_recv_verdict(struct sock *ctnl, struct sk_buff *skb,
 			verdict = NF_DROP;
 
 		if (ct)
-			nfqnl_ct_seq_adjust(skb, ct, ctinfo, diff);
+			nfqnl_ct_seq_adjust(entry->skb, ct, ctinfo, diff);
 	}
 
 	if (nfqa[NFQA_MARK])
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 5/7] netfilter: ipset: Validate the set family and not the set type family at swapping
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

This closes netfilter bugzilla #843, reported by Quentin Armitage.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index c8c303c..f2e30fb 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1052,7 +1052,7 @@ ip_set_swap(struct sock *ctnl, struct sk_buff *skb,
 	 * Not an artificial restriction anymore, as we must prevent
 	 * possible loops created by swapping in setlist type of sets. */
 	if (!(from->type->features == to->type->features &&
-	      from->type->family == to->type->family))
+	      from->family == to->family))
 		return -IPSET_ERR_TYPE_MISMATCH;
 
 	strncpy(from_name, from->name, IPSET_MAXNAMELEN);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 6/7] netfilter: ipset: Fix serious failure in CIDR tracking
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This fixes a serious bug affecting all hash types with a net element -
specifically, if a CIDR value is deleted such that none of the same size
exist any more, all larger (less-specific) values will then fail to
match. Adding back any prefix with a CIDR equal to or more specific than
the one deleted will fix it.

Steps to reproduce:
ipset -N test hash:net
ipset -A test 1.1.0.0/16
ipset -A test 2.2.2.0/24
ipset -T test 1.1.1.1           #1.1.1.1 IS in set
ipset -D test 2.2.2.0/24
ipset -T test 1.1.1.1           #1.1.1.1 IS NOT in set

This is due to the fact that the nets counter was unconditionally
decremented prior to the iteration that shifts up the entries. Now, we
first check if there is a proceeding entry and if not, decrement it and
return. Otherwise, we proceed to iterate and then zero the last element,
which, in most cases, will already be zero.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 net/netfilter/ipset/ip_set_hash_gen.h |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index 57beb17..707bc52 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -325,18 +325,22 @@ mtype_add_cidr(struct htype *h, u8 cidr, u8 nets_length)
 static void
 mtype_del_cidr(struct htype *h, u8 cidr, u8 nets_length)
 {
-	u8 i, j;
-
-	for (i = 0; i < nets_length - 1 && h->nets[i].cidr != cidr; i++)
-		;
-	h->nets[i].nets--;
-
-	if (h->nets[i].nets != 0)
-		return;
-
-	for (j = i; j < nets_length - 1 && h->nets[j].nets; j++) {
-		h->nets[j].cidr = h->nets[j + 1].cidr;
-		h->nets[j].nets = h->nets[j + 1].nets;
+	u8 i, j, net_end = nets_length - 1;
+
+	for (i = 0; i < nets_length; i++) {
+	        if (h->nets[i].cidr != cidr)
+	                continue;
+                if (h->nets[i].nets > 1 || i == net_end ||
+                    h->nets[i + 1].nets == 0) {
+                        h->nets[i].nets--;
+                        return;
+                }
+                for (j = i; j < net_end && h->nets[j].nets; j++) {
+		        h->nets[j].cidr = h->nets[j + 1].cidr;
+		        h->nets[j].nets = h->nets[j + 1].nets;
+                }
+                h->nets[j].nets = 0;
+                return;
 	}
 }
 #endif
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/7] netfilter: nf_nat_proto_icmpv6:: fix wrong comparison in icmpv6_manip_pkt
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Phil Oester <kernel@linuxace.com>

In commit 58a317f1 (netfilter: ipv6: add IPv6 NAT support), icmpv6_manip_pkt
was added with an incorrect comparison of ICMP codes to types.  This causes
problems when using NAT rules with the --random option.  Correct the
comparison.

This closes netfilter bugzilla #851, reported by Alexander Neumann.

Signed-off-by: Phil Oester <kernel@linuxace.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/ipv6/netfilter/nf_nat_proto_icmpv6.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/netfilter/nf_nat_proto_icmpv6.c b/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
index 61aaf70..2205e8e 100644
--- a/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
+++ b/net/ipv6/netfilter/nf_nat_proto_icmpv6.c
@@ -69,8 +69,8 @@ icmpv6_manip_pkt(struct sk_buff *skb,
 	hdr = (struct icmp6hdr *)(skb->data + hdroff);
 	l3proto->csum_update(skb, iphdroff, &hdr->icmp6_cksum,
 			     tuple, maniptype);
-	if (hdr->icmp6_code == ICMPV6_ECHO_REQUEST ||
-	    hdr->icmp6_code == ICMPV6_ECHO_REPLY) {
+	if (hdr->icmp6_type == ICMPV6_ECHO_REQUEST ||
+	    hdr->icmp6_type == ICMPV6_ECHO_REPLY) {
 		inet_proto_csum_replace2(&hdr->icmp6_cksum, skb,
 					 hdr->icmp6_identifier,
 					 tuple->src.u.icmp.id, 0);
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 4/7] netfilter: ipset: Consistent userspace testing with nomatch flag
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1379455654-3905-1-git-send-email-pablo@netfilter.org>

From: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>

The "nomatch" commandline flag should invert the matching at testing,
similarly to the --return-nomatch flag of the "set" match of iptables.
Until now it worked with the elements with "nomatch" flag only. From
now on it works with elements without the flag too, i.e:

 # ipset n test hash:net
 # ipset a test 10.0.0.0/24 nomatch
 # ipset t test 10.0.0.1
 10.0.0.1 is NOT in set test.
 # ipset t test 10.0.0.1 nomatch
 10.0.0.1 is in set test.

 # ipset a test 192.168.0.0/24
 # ipset t test 192.168.0.1
 192.168.0.1 is in set test.
 # ipset t test 192.168.0.1 nomatch
 192.168.0.1 is NOT in set test.

 Before the patch the results were

 ...
 # ipset t test 192.168.0.1
 192.168.0.1 is in set test.
 # ipset t test 192.168.0.1 nomatch
 192.168.0.1 is in set test.

Signed-off-by: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
---
 include/linux/netfilter/ipset/ip_set.h      |    6 ++++--
 net/netfilter/ipset/ip_set_core.c           |    3 +--
 net/netfilter/ipset/ip_set_hash_ipportnet.c |    4 ++--
 net/netfilter/ipset/ip_set_hash_net.c       |    4 ++--
 net/netfilter/ipset/ip_set_hash_netiface.c  |    4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c   |    4 ++--
 6 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index d80e275..9ac9fbd 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -296,10 +296,12 @@ ip_set_eexist(int ret, u32 flags)
 
 /* Match elements marked with nomatch */
 static inline bool
-ip_set_enomatch(int ret, u32 flags, enum ipset_adt adt)
+ip_set_enomatch(int ret, u32 flags, enum ipset_adt adt, struct ip_set *set)
 {
 	return adt == IPSET_TEST &&
-	       ret == -ENOTEMPTY && ((flags >> 16) & IPSET_FLAG_NOMATCH);
+	       (set->type->features & IPSET_TYPE_NOMATCH) &&
+	       ((flags >> 16) & IPSET_FLAG_NOMATCH) &&
+	       (ret > 0 || ret == -ENOTEMPTY);
 }
 
 /* Check the NLA_F_NET_BYTEORDER flag */
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index f771390..c8c303c 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -1489,8 +1489,7 @@ ip_set_utest(struct sock *ctnl, struct sk_buff *skb,
 	if (ret == -EAGAIN)
 		ret = 1;
 
-	return (ret < 0 && ret != -ENOTEMPTY) ? ret :
-		ret > 0 ? 0 : -IPSET_ERR_EXIST;
+	return ret > 0 ? 0 : -IPSET_ERR_EXIST;
 }
 
 /* Get headed data of a set */
diff --git a/net/netfilter/ipset/ip_set_hash_ipportnet.c b/net/netfilter/ipset/ip_set_hash_ipportnet.c
index c6a5253..f15f3e2 100644
--- a/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -260,7 +260,7 @@ hash_ipportnet4_uadt(struct ip_set *set, struct nlattr *tb[],
 		e.ip = htonl(ip);
 		e.ip2 = htonl(ip2_from & ip_set_hostmask(e.cidr + 1));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -544,7 +544,7 @@ hash_ipportnet6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
diff --git a/net/netfilter/ipset/ip_set_hash_net.c b/net/netfilter/ipset/ip_set_hash_net.c
index da740ce..223e9f5 100644
--- a/net/netfilter/ipset/ip_set_hash_net.c
+++ b/net/netfilter/ipset/ip_set_hash_net.c
@@ -199,7 +199,7 @@ hash_net4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !tb[IPSET_ATTR_IP_TO]) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret:
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -396,7 +396,7 @@ hash_net6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ret = adtfn(set, &e, &ext, &ext, flags);
 
-	return ip_set_enomatch(ret, flags, adt) ? 1 :
+	return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 	       ip_set_eexist(ret, flags) ? 0 : ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_netiface.c b/net/netfilter/ipset/ip_set_hash_netiface.c
index 84ae6f6..7d798d5 100644
--- a/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -368,7 +368,7 @@ hash_netiface4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !tb[IPSET_ATTR_IP_TO]) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -634,7 +634,7 @@ hash_netiface6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	ret = adtfn(set, &e, &ext, &ext, flags);
 
-	return ip_set_enomatch(ret, flags, adt) ? 1 :
+	return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 	       ip_set_eexist(ret, flags) ? 0 : ret;
 }
 
diff --git a/net/netfilter/ipset/ip_set_hash_netport.c b/net/netfilter/ipset/ip_set_hash_netport.c
index 9a08698..09d6690 100644
--- a/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/net/netfilter/ipset/ip_set_hash_netport.c
@@ -244,7 +244,7 @@ hash_netport4_uadt(struct ip_set *set, struct nlattr *tb[],
 	if (adt == IPSET_TEST || !(with_ports || tb[IPSET_ATTR_IP_TO])) {
 		e.ip = htonl(ip & ip_set_hostmask(e.cidr + 1));
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
@@ -489,7 +489,7 @@ hash_netport6_uadt(struct ip_set *set, struct nlattr *tb[],
 
 	if (adt == IPSET_TEST || !with_ports || !tb[IPSET_ATTR_PORT_TO]) {
 		ret = adtfn(set, &e, &ext, &ext, flags);
-		return ip_set_enomatch(ret, flags, adt) ? 1 :
+		return ip_set_enomatch(ret, flags, adt, set) ? -ret :
 		       ip_set_eexist(ret, flags) ? 0 : ret;
 	}
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 0/7] netfilter fixes for net
From: Pablo Neira Ayuso @ 2013-09-17 22:07 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Hi David,

The following patchset contains Netfilter fixes for you net tree,
they are:

* Fix ICMPv6 NAT due to wrong comparison, code instead of type, from
  Phil Oester.

* Fix RCU race in conntrack extensions release path, from Michal Kubecek.

* Fix missing inversion in the userspace ipset test command match if
  the nomatch option is specified, from Jozsef Kadlecsik.

* Skip layer 4 protocol matching in ipset in case of IPv6 fragments,
  also from Jozsef Kadlecsik.

* Fix sequence adjustment in nfnetlink_queue due to using the netlink
  skb instead of the network skb, from Gao feng.

* Make sure we cannot swap of sets with different layer 3 family in
  ipset, from Jozsef Kadlecsik.

* Fix possible bogus matching in ipset if hash sets with net elements
  are used, from Oliver Smith.

Gao feng (1):
  netfilter: nfnetlink_queue: use network skb for sequence adjustment

Jozsef Kadlecsik (3):
  netfilter: ipset: Skip really non-first fragments for IPv6 when getting port/protocol
  netfilter: ipset: Consistent userspace testing with nomatch flag
  netfilter: ipset: Validate the set family and not the set type family at swapping

Michal Kubeček (1):
  netfilter: nf_conntrack: use RCU safe kfree for conntrack extensions

Oliver Smith (1):
  netfilter: ipset: Fix serious failure in CIDR tracking

Phil Oester (1):
  netfilter: nf_nat_proto_icmpv6:: fix wrong comparison in icmpv6_manip_pkt

 include/linux/netfilter/ipset/ip_set.h      |    6 ++++--
 include/net/netfilter/nf_conntrack_extend.h |    2 +-
 net/ipv6/netfilter/nf_nat_proto_icmpv6.c    |    4 ++--
 net/netfilter/ipset/ip_set_core.c           |    5 ++---
 net/netfilter/ipset/ip_set_getport.c        |    4 ++--
 net/netfilter/ipset/ip_set_hash_gen.h       |   28 +++++++++++++++------------
 net/netfilter/ipset/ip_set_hash_ipportnet.c |    4 ++--
 net/netfilter/ipset/ip_set_hash_net.c       |    4 ++--
 net/netfilter/ipset/ip_set_hash_netiface.c  |    4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c   |    4 ++--
 net/netfilter/nfnetlink_queue_core.c        |    2 +-
 11 files changed, 36 insertions(+), 31 deletions(-)

-- 
1.7.10.4

^ permalink raw reply

* [PATCH 0/7] netfilter fixes for net
From: Pablo Neira Ayuso @ 2013-09-17 22:21 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev

Resending pull request email, previous one was missing the pull request
information itself, sorry.

--

Hi David,

The following patchset contains Netfilter fixes for you net tree,
mostly targeted to ipset, they are:

* Fix ICMPv6 NAT due to wrong comparison, code instead of type, from
  Phil Oester.

* Fix RCU race in conntrack extensions release path, from Michal Kubecek.

* Fix missing inversion in the userspace ipset test command match if
  the nomatch option is specified, from Jozsef Kadlecsik.

* Skip layer 4 protocol matching in ipset in case of IPv6 fragments,
  also from Jozsef Kadlecsik.

* Fix sequence adjustment in nfnetlink_queue due to using the netlink
  skb instead of the network skb, from Gao feng.

* Make sure we cannot swap of sets with different layer 3 family in
  ipset, from Jozsef Kadlecsik.

* Fix possible bogus matching in ipset if hash sets with net elements
  are used, from Oliver Smith.

You can pull these changes from:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master

Thanks!

----------------------------------------------------------------

The following changes since commit c19d65c95c6d472d69829fea7d473228493d5245:

  bnx2x: Fix configuration of doorbell block (2013-09-09 17:06:14 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git master

for you to fetch changes up to 0a0d80eb39aa465b7bdf6f7754d0ba687eb3d2a7:

  netfilter: nfnetlink_queue: use network skb for sequence adjustment (2013-09-17 13:05:12 +0200)

----------------------------------------------------------------
Gao feng (1):
      netfilter: nfnetlink_queue: use network skb for sequence adjustment

Jozsef Kadlecsik (3):
      netfilter: ipset: Skip really non-first fragments for IPv6 when getting port/protocol
      netfilter: ipset: Consistent userspace testing with nomatch flag
      netfilter: ipset: Validate the set family and not the set type family at swapping

Michal Kubeček (1):
      netfilter: nf_conntrack: use RCU safe kfree for conntrack extensions

Oliver Smith (1):
      netfilter: ipset: Fix serious failure in CIDR tracking

Phil Oester (1):
      netfilter: nf_nat_proto_icmpv6:: fix wrong comparison in icmpv6_manip_pkt

 include/linux/netfilter/ipset/ip_set.h      |    6 ++++--
 include/net/netfilter/nf_conntrack_extend.h |    2 +-
 net/ipv6/netfilter/nf_nat_proto_icmpv6.c    |    4 ++--
 net/netfilter/ipset/ip_set_core.c           |    5 ++---
 net/netfilter/ipset/ip_set_getport.c        |    4 ++--
 net/netfilter/ipset/ip_set_hash_gen.h       |   28 +++++++++++++++------------
 net/netfilter/ipset/ip_set_hash_ipportnet.c |    4 ++--
 net/netfilter/ipset/ip_set_hash_net.c       |    4 ++--
 net/netfilter/ipset/ip_set_hash_netiface.c  |    4 ++--
 net/netfilter/ipset/ip_set_hash_netport.c   |    4 ++--
 net/netfilter/nfnetlink_queue_core.c        |    2 +-
 11 files changed, 36 insertions(+), 31 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* add ip to interface as primary ip address, not secondary ip when there is alreadly primary ip?
From: Vincent Li @ 2013-09-17 22:31 UTC (permalink / raw)
  To: netdev@vger.kernel.org

Hi,

we need to add  more than one ip address to one interface, but want to
be able to specify which ip address to be primary and which to be
secondary (be deterministic) , what we have found out is that the
first ip address added to the interface will be always the primary ip
address, the primary or secondary is depending on which ip get added
to the interface first.

we could also delete an existing primary ip address, then the
secondary ip will be promoted to primary.

ideally though, we would like to have the capability in
kernel/userspace  to specify a flag or something to add any ip as
primary even if there is already existing primary ip address (could
downgrade the existing primary to secondary ip). does this make sense
?

Vincent

^ permalink raw reply

* [PATCH net] ip: generate unique IP identificator if local fragmentation is allowed
From: Ansis Atteka @ 2013-09-17 22:25 UTC (permalink / raw)
  To: netdev; +Cc: Ansis Atteka

If local fragmentation is allowed, then ip_select_ident() and
ip_select_ident_more() need to generate unique IDs to ensure
correct defragmentation on the peer.

For example, if IPsec (tunnel mode) has to encrypt large skbs
that have local_df bit set, then all IP fragments that belonged
to different ESP datagrams would have used the same identificator.
If one of these IP fragments would get lost or reordered, then
peer could possibly stitch together wrong IP fragments that did
not belong to the same datagram. This would lead to a packet loss
or data corruption.

Signed-off-by: Ansis Atteka <aatteka@nicira.com>
---
 drivers/net/ppp/pptp.c          |    2 +-
 include/net/ip.h                |   12 ++++++++----
 net/ipv4/igmp.c                 |    4 ++--
 net/ipv4/inetpeer.c             |    4 ++--
 net/ipv4/ip_output.c            |    7 ++++---
 net/ipv4/ipmr.c                 |    2 +-
 net/ipv4/raw.c                  |    2 +-
 net/ipv4/xfrm4_mode_tunnel.c    |    2 +-
 net/netfilter/ipvs/ip_vs_xmit.c |    2 +-
 9 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/net/ppp/pptp.c b/drivers/net/ppp/pptp.c
index 6fa5ae0..0180531 100644
--- a/drivers/net/ppp/pptp.c
+++ b/drivers/net/ppp/pptp.c
@@ -281,7 +281,7 @@ static int pptp_xmit(struct ppp_channel *chan, struct sk_buff *skb)
 	nf_reset(skb);
 
 	skb->ip_summed = CHECKSUM_NONE;
-	ip_select_ident(iph, &rt->dst, NULL);
+	ip_select_ident(skb, &rt->dst, NULL);
 	ip_send_check(iph);
 
 	ip_local_out(skb);
diff --git a/include/net/ip.h b/include/net/ip.h
index 48f5597..5e52688 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -264,9 +264,11 @@ int ip_dont_fragment(struct sock *sk, struct dst_entry *dst)
 
 extern void __ip_select_ident(struct iphdr *iph, struct dst_entry *dst, int more);
 
-static inline void ip_select_ident(struct iphdr *iph, struct dst_entry *dst, struct sock *sk)
+static inline void ip_select_ident(struct sk_buff *skb, struct dst_entry *dst, struct sock *sk)
 {
-	if (iph->frag_off & htons(IP_DF)) {
+	struct iphdr *iph = ip_hdr(skb);
+
+	if ((iph->frag_off & htons(IP_DF)) && !skb->local_df) {
 		/* This is only to work around buggy Windows95/2000
 		 * VJ compression implementations.  If the ID field
 		 * does not change, they drop every other packet in
@@ -278,9 +280,11 @@ static inline void ip_select_ident(struct iphdr *iph, struct dst_entry *dst, str
 		__ip_select_ident(iph, dst, 0);
 }
 
-static inline void ip_select_ident_more(struct iphdr *iph, struct dst_entry *dst, struct sock *sk, int more)
+static inline void ip_select_ident_more(struct sk_buff *skb, struct dst_entry *dst, struct sock *sk, int more)
 {
-	if (iph->frag_off & htons(IP_DF)) {
+	struct iphdr *iph = ip_hdr(skb);
+
+	if ((iph->frag_off & htons(IP_DF)) && !skb->local_df) {
 		if (sk && inet_sk(sk)->inet_daddr) {
 			iph->id = htons(inet_sk(sk)->inet_id);
 			inet_sk(sk)->inet_id += 1 + more;
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index d6c0e64..dace87f 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -369,7 +369,7 @@ static struct sk_buff *igmpv3_newpack(struct net_device *dev, int size)
 	pip->saddr    = fl4.saddr;
 	pip->protocol = IPPROTO_IGMP;
 	pip->tot_len  = 0;	/* filled in later */
-	ip_select_ident(pip, &rt->dst, NULL);
+	ip_select_ident(skb, &rt->dst, NULL);
 	((u8 *)&pip[1])[0] = IPOPT_RA;
 	((u8 *)&pip[1])[1] = 4;
 	((u8 *)&pip[1])[2] = 0;
@@ -714,7 +714,7 @@ static int igmp_send_report(struct in_device *in_dev, struct ip_mc_list *pmc,
 	iph->daddr    = dst;
 	iph->saddr    = fl4.saddr;
 	iph->protocol = IPPROTO_IGMP;
-	ip_select_ident(iph, &rt->dst, NULL);
+	ip_select_ident(skb, &rt->dst, NULL);
 	((u8 *)&iph[1])[0] = IPOPT_RA;
 	((u8 *)&iph[1])[1] = 4;
 	((u8 *)&iph[1])[2] = 0;
diff --git a/net/ipv4/inetpeer.c b/net/ipv4/inetpeer.c
index 000e3d2..33d5537 100644
--- a/net/ipv4/inetpeer.c
+++ b/net/ipv4/inetpeer.c
@@ -32,8 +32,8 @@
  *  At the moment of writing this notes identifier of IP packets is generated
  *  to be unpredictable using this code only for packets subjected
  *  (actually or potentially) to defragmentation.  I.e. DF packets less than
- *  PMTU in size uses a constant ID and do not use this code (see
- *  ip_select_ident() in include/net/ip.h).
+ *  PMTU in size when local fragmentation is disabled use a constant ID and do
+ *  not use this code (see ip_select_ident() in include/net/ip.h).
  *
  *  Route cache entries hold references to our nodes.
  *  New cache entries get references via lookup by destination IP address in
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 9ee17e3..f9c4fbb 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -148,7 +148,7 @@ int ip_build_and_send_pkt(struct sk_buff *skb, struct sock *sk,
 	iph->daddr    = (opt && opt->opt.srr ? opt->opt.faddr : daddr);
 	iph->saddr    = saddr;
 	iph->protocol = sk->sk_protocol;
-	ip_select_ident(iph, &rt->dst, sk);
+	ip_select_ident(skb, &rt->dst, sk);
 
 	if (opt && opt->opt.optlen) {
 		iph->ihl += opt->opt.optlen>>2;
@@ -386,7 +386,7 @@ packet_routed:
 		ip_options_build(skb, &inet_opt->opt, inet->inet_daddr, rt, 0);
 	}
 
-	ip_select_ident_more(iph, &rt->dst, sk,
+	ip_select_ident_more(skb, &rt->dst, sk,
 			     (skb_shinfo(skb)->gso_segs ?: 1) - 1);
 
 	skb->priority = sk->sk_priority;
@@ -1317,6 +1317,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
 		ttl = ip_select_ttl(inet, &rt->dst);
 
 	iph = (struct iphdr *)skb->data;
+
 	iph->version = 4;
 	iph->ihl = 5;
 	iph->tos = inet->tos;
@@ -1324,7 +1325,7 @@ struct sk_buff *__ip_make_skb(struct sock *sk,
 	iph->ttl = ttl;
 	iph->protocol = sk->sk_protocol;
 	ip_copy_addrs(iph, fl4);
-	ip_select_ident(iph, &rt->dst, sk);
+	ip_select_ident(skb, &rt->dst, sk);
 
 	if (opt) {
 		iph->ihl += opt->optlen>>2;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 9ae54b0..62212c7 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1658,7 +1658,7 @@ static void ip_encap(struct sk_buff *skb, __be32 saddr, __be32 daddr)
 	iph->protocol	=	IPPROTO_IPIP;
 	iph->ihl	=	5;
 	iph->tot_len	=	htons(skb->len);
-	ip_select_ident(iph, skb_dst(skb), NULL);
+	ip_select_ident(skb, skb_dst(skb), NULL);
 	ip_send_check(iph);
 
 	memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt));
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index a86c7ae..bfec521 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -387,7 +387,7 @@ static int raw_send_hdrinc(struct sock *sk, struct flowi4 *fl4,
 		iph->check   = 0;
 		iph->tot_len = htons(length);
 		if (!iph->id)
-			ip_select_ident(iph, &rt->dst, NULL);
+			ip_select_ident(skb, &rt->dst, NULL);
 
 		iph->check = ip_fast_csum((unsigned char *)iph, iph->ihl);
 	}
diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c
index eb1dd4d..b5663c3 100644
--- a/net/ipv4/xfrm4_mode_tunnel.c
+++ b/net/ipv4/xfrm4_mode_tunnel.c
@@ -117,7 +117,7 @@ static int xfrm4_mode_tunnel_output(struct xfrm_state *x, struct sk_buff *skb)
 
 	top_iph->frag_off = (flags & XFRM_STATE_NOPMTUDISC) ?
 		0 : (XFRM_MODE_SKB_CB(skb)->frag_off & htons(IP_DF));
-	ip_select_ident(top_iph, dst->child, NULL);
+	ip_select_ident(skb, dst->child, NULL);
 
 	top_iph->ttl = ip4_dst_hoplimit(dst->child);
 
diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c
index b75ff64..c47444e 100644
--- a/net/netfilter/ipvs/ip_vs_xmit.c
+++ b/net/netfilter/ipvs/ip_vs_xmit.c
@@ -883,7 +883,7 @@ ip_vs_tunnel_xmit(struct sk_buff *skb, struct ip_vs_conn *cp,
 	iph->daddr		=	cp->daddr.ip;
 	iph->saddr		=	saddr;
 	iph->ttl		=	old_iph->ttl;
-	ip_select_ident(iph, &rt->dst, NULL);
+	ip_select_ident(skb, &rt->dst, NULL);
 
 	/* Another hack: avoid icmp_send in ip_fragment */
 	skb->local_df = 1;
-- 
1.7.9.5

^ permalink raw reply related

* Re: Potential out-of-bounds access in ip6_finish_output2
From: Hannes Frederic Sowa @ 2013-09-17 22:48 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: yoshfuji, netdev, Paul Turner, Andrey Konovalov,
	Kostya Serebryany, Tom Herbert
In-Reply-To: <CACT4Y+ZXqoixvCyN5703+sVhCV4ND-vNwash2Z1vSDoLgKNyVg@mail.gmail.com>

On Mon, Sep 16, 2013 at 10:13:10PM -0700, Dmitry Vyukov wrote:
> Hi,
> 
> I am working on AddressSanitizer -- a tool that detects use-after-free
> and out-of-bounds bugs
> (https://code.google.com/p/address-sanitizer/wiki/AddressSanitizerForKernel).
> 
> I've got a dozen of reports in ip6_finish_output2. Below are 2 of
> them. They are always followed by kernel crash. Unfortunately I don't
> have a reproducer because I am using trinity fuzzer. I would
> appreciate if somebody familiar with the code look at sources and
> maybe spot the bug.

Thanks for the report!

I tried reproducing the bug and hit some other bugs nearby. I'll try to fix
them, but this could take some time.

Greetings,

  Hannes

^ 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