Netdev List
 help / color / mirror / Atom feed
* [patch net-next 3/4] rtnl: expose carrier value with possibility to set it
From: Jiri Pirko @ 2012-12-18 22:14 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, mirqus, shemminger, greearb, fbl,
	john.r.fastabend
In-Reply-To: <1355868858-1713-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 Documentation/networking/operstates.txt |  4 ++++
 include/uapi/linux/if_link.h            |  1 +
 net/core/rtnetlink.c                    | 10 ++++++++++
 3 files changed, 15 insertions(+)

diff --git a/Documentation/networking/operstates.txt b/Documentation/networking/operstates.txt
index 1a77a3c..9769457 100644
--- a/Documentation/networking/operstates.txt
+++ b/Documentation/networking/operstates.txt
@@ -88,6 +88,10 @@ set this flag. On netif_carrier_off(), the scheduler stops sending
 packets. The name 'carrier' and the inversion are historical, think of
 it as lower layer.
 
+Note that for certain kind of soft-devices, which are not managing any
+real hardware, there is possible to set this bit from userpsace.
+One should use TVL IFLA_CARRIER to do so.
+
 netif_carrier_ok() can be used to query that bit.
 
 __LINK_STATE_DORMANT, maps to IFF_DORMANT:
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 60f3b6b..c4edfe1 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -142,6 +142,7 @@ enum {
 #define IFLA_PROMISCUITY IFLA_PROMISCUITY
 	IFLA_NUM_TX_QUEUES,
 	IFLA_NUM_RX_QUEUES,
+	IFLA_CARRIER,
 	__IFLA_MAX
 };
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 1868625..2ef7a56 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -780,6 +780,7 @@ static noinline size_t if_nlmsg_size(const struct net_device *dev,
 	       + nla_total_size(4) /* IFLA_MTU */
 	       + nla_total_size(4) /* IFLA_LINK */
 	       + nla_total_size(4) /* IFLA_MASTER */
+	       + nla_total_size(1) /* IFLA_CARRIER */
 	       + nla_total_size(4) /* IFLA_PROMISCUITY */
 	       + nla_total_size(4) /* IFLA_NUM_TX_QUEUES */
 	       + nla_total_size(4) /* IFLA_NUM_RX_QUEUES */
@@ -909,6 +910,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
 	     nla_put_u32(skb, IFLA_LINK, dev->iflink)) ||
 	    (dev->master &&
 	     nla_put_u32(skb, IFLA_MASTER, dev->master->ifindex)) ||
+	    nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
 	    (dev->qdisc &&
 	     nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
 	    (dev->ifalias &&
@@ -1108,6 +1110,7 @@ const struct nla_policy ifla_policy[IFLA_MAX+1] = {
 	[IFLA_MTU]		= { .type = NLA_U32 },
 	[IFLA_LINK]		= { .type = NLA_U32 },
 	[IFLA_MASTER]		= { .type = NLA_U32 },
+	[IFLA_CARRIER]		= { .type = NLA_U8 },
 	[IFLA_TXQLEN]		= { .type = NLA_U32 },
 	[IFLA_WEIGHT]		= { .type = NLA_U32 },
 	[IFLA_OPERSTATE]	= { .type = NLA_U8 },
@@ -1438,6 +1441,13 @@ static int do_setlink(struct net_device *dev, struct ifinfomsg *ifm,
 		modified = 1;
 	}
 
+	if (tb[IFLA_CARRIER]) {
+		err = dev_change_carrier(dev, nla_get_u8(tb[IFLA_CARRIER]));
+		if (err)
+			goto errout;
+		modified = 1;
+	}
+
 	if (tb[IFLA_TXQLEN])
 		dev->tx_queue_len = nla_get_u32(tb[IFLA_TXQLEN]);
 
-- 
1.8.0

^ permalink raw reply related

* [patch net-next 4/4] dummy: implement carrier change
From: Jiri Pirko @ 2012-12-18 22:14 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, bhutchings, mirqus, shemminger, greearb, fbl,
	john.r.fastabend
In-Reply-To: <1355868858-1713-1-git-send-email-jiri@resnulli.us>

Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
 drivers/net/dummy.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index c260af5..42aa54a 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -100,6 +100,15 @@ static void dummy_dev_uninit(struct net_device *dev)
 	free_percpu(dev->dstats);
 }
 
+static int dummy_change_carrier(struct net_device *dev, bool new_carrier)
+{
+	if (new_carrier)
+		netif_carrier_on(dev);
+	else
+		netif_carrier_off(dev);
+	return 0;
+}
+
 static const struct net_device_ops dummy_netdev_ops = {
 	.ndo_init		= dummy_dev_init,
 	.ndo_uninit		= dummy_dev_uninit,
@@ -108,6 +117,7 @@ static const struct net_device_ops dummy_netdev_ops = {
 	.ndo_set_rx_mode	= set_multicast_list,
 	.ndo_set_mac_address	= eth_mac_addr,
 	.ndo_get_stats64	= dummy_get_stats64,
+	.ndo_change_carrier	= dummy_change_carrier,
 };
 
 static void dummy_setup(struct net_device *dev)
-- 
1.8.0

^ permalink raw reply related

* Re: [PATCH V2 00/12] Add basic VLAN support to bridges
From: Jiri Pirko @ 2012-12-18 22:32 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
In-Reply-To: <1355857263-31197-1-git-send-email-vyasevic@redhat.com>



I see that this patchset replicates a lot of code which is already
present in net/8021q/ or include/linux/if_vlan.h. I think it would
be nice to move this code into some "common" place, wouldn't it?

Jiri

Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>This series of patches provides an ability to add VLANs to the bridge
>ports.  This is similar to what can be found in most switches.  The bridge
>port may have any number of VLANs added to it including vlan 0 priority tagged
>traffic.  When vlans are added to the port, only traffic tagged with particular
>vlan will forwarded over this port.  Additionally, vlan ids are added to FDB
>entries and become part of the lookup.  This way we correctly identify the FDB
>entry.
>
>A single vlan may also be designated as untagged.  Any untagged traffic
>recieved by the port will be assigned to this vlan.  Any traffic exiting
>the port with a VID matching the untagged vlan will exit untagged (the
>bridge will strip the vlan header).  This is similar to "Native Vlan" support
>available in most switches.
>
>The default behavior ofthe bridge is unchanged if no vlans have been
>configured.
>
>Changes since v1:
> - Fixed some forwarding bugs.
> - Add vlan to local fdb entries.  New local entries are created per vlan
>   to facilite correct forwarding to bridge interface.
> - Allow configuration of vlans directly on the bridge master device
>   in addition to ports.
>
>Changes since rfc v2:
> - Per-port vlan bitmap is gone and is replaced with a vlan list.
> - Added bridge vlan list, which is referenced by each port.  Entries in
>   the birdge vlan list have port bitmap that shows which port are parts
>   of which vlan.
> - Netlink API changes.
> - Dropped sysfs support for now.  If people think this is really usefull,
>   can add it back.
> - Support for native/untagged vlans.
>
>Changes since rfc v1:
> - Comments addressed regarding formatting and RCU usage
> - iocts have been removed and changed over the netlink interface.
> - Added support of user added ndb entries.
> - changed sysfs interface to export a bitmap.  Also added a write interface.
>   I am not sure how much I like it, but it made my testing easier/faster.  I
>   might change the write interface to take text instead of binary.
>
>
>Vlad Yasevich (12):
>  bridge: Add vlan filtering infrastructure
>  bridge: Validate that vlan is permitted on ingress
>  bridge: Verify that a vlan is allowed to egress on give port
>  bridge: Cache vlan in the cb for faster egress lookup.
>  bridge: Add vlan to unicast fdb entries
>  bridge: Add vlan id to multicast groups
>  bridge: Add netlink interface to configure vlans on bridge ports
>  bridge: Add vlan support to static neighbors
>  bridge: Add the ability to configure untagged vlans
>  bridge: Implement untagged vlan handling
>  bridge: Dump vlan information from a bridge port
>  bridge: Add vlan support for local fdb entries
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    5 +-
> drivers/net/macvlan.c                         |    2 +-
> drivers/net/vxlan.c                           |    3 +-
> include/linux/netdevice.h                     |    4 +-
> include/uapi/linux/if_bridge.h                |   23 ++-
> include/uapi/linux/neighbour.h                |    1 +
> include/uapi/linux/rtnetlink.h                |    1 +
> net/bridge/br_device.c                        |   34 ++-
> net/bridge/br_fdb.c                           |  253 ++++++++++++---
> net/bridge/br_forward.c                       |  160 ++++++++++
> net/bridge/br_if.c                            |  404 ++++++++++++++++++++++++-
> net/bridge/br_input.c                         |   65 ++++-
> net/bridge/br_multicast.c                     |   71 +++--
> net/bridge/br_netlink.c                       |  178 ++++++++++--
> net/bridge/br_private.h                       |   71 ++++-
> net/core/rtnetlink.c                          |   40 ++-
> 16 files changed, 1190 insertions(+), 125 deletions(-)
>
>-- 
>1.7.7.6
>
>--
>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 V2 00/12] Add basic VLAN support to bridges
From: Vlad Yasevich @ 2012-12-18 22:46 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, shemminger, davem, or.gerlitz, jhs, mst
In-Reply-To: <20121218223244.GC1690@minipsycho.orion>

On 12/18/2012 05:32 PM, Jiri Pirko wrote:
>
>
> I see that this patchset replicates a lot of code which is already
> present in net/8021q/ or include/linux/if_vlan.h. I think it would
> be nice to move this code into some "common" place, wouldn't it?
>

The only replication that I am aware of is in br_vlan_untag().  I 
thought about pulling that piece out, but I think there is a reason
why it's not available when 801q support isn't turned on.  I noted that
openvswitch implemented its own vlan header manipulation functions as well.

What else are you seeing that's duplicate?

-vlad

> Jiri
>
> Tue, Dec 18, 2012 at 08:00:51PM CET, vyasevic@redhat.com wrote:
>> This series of patches provides an ability to add VLANs to the bridge
>> ports.  This is similar to what can be found in most switches.  The bridge
>> port may have any number of VLANs added to it including vlan 0 priority tagged
>> traffic.  When vlans are added to the port, only traffic tagged with particular
>> vlan will forwarded over this port.  Additionally, vlan ids are added to FDB
>> entries and become part of the lookup.  This way we correctly identify the FDB
>> entry.
>>
>> A single vlan may also be designated as untagged.  Any untagged traffic
>> recieved by the port will be assigned to this vlan.  Any traffic exiting
>> the port with a VID matching the untagged vlan will exit untagged (the
>> bridge will strip the vlan header).  This is similar to "Native Vlan" support
>> available in most switches.
>>
>> The default behavior ofthe bridge is unchanged if no vlans have been
>> configured.
>>
>> Changes since v1:
>> - Fixed some forwarding bugs.
>> - Add vlan to local fdb entries.  New local entries are created per vlan
>>    to facilite correct forwarding to bridge interface.
>> - Allow configuration of vlans directly on the bridge master device
>>    in addition to ports.
>>
>> Changes since rfc v2:
>> - Per-port vlan bitmap is gone and is replaced with a vlan list.
>> - Added bridge vlan list, which is referenced by each port.  Entries in
>>    the birdge vlan list have port bitmap that shows which port are parts
>>    of which vlan.
>> - Netlink API changes.
>> - Dropped sysfs support for now.  If people think this is really usefull,
>>    can add it back.
>> - Support for native/untagged vlans.
>>
>> Changes since rfc v1:
>> - Comments addressed regarding formatting and RCU usage
>> - iocts have been removed and changed over the netlink interface.
>> - Added support of user added ndb entries.
>> - changed sysfs interface to export a bitmap.  Also added a write interface.
>>    I am not sure how much I like it, but it made my testing easier/faster.  I
>>    might change the write interface to take text instead of binary.
>>
>>
>> Vlad Yasevich (12):
>>   bridge: Add vlan filtering infrastructure
>>   bridge: Validate that vlan is permitted on ingress
>>   bridge: Verify that a vlan is allowed to egress on give port
>>   bridge: Cache vlan in the cb for faster egress lookup.
>>   bridge: Add vlan to unicast fdb entries
>>   bridge: Add vlan id to multicast groups
>>   bridge: Add netlink interface to configure vlans on bridge ports
>>   bridge: Add vlan support to static neighbors
>>   bridge: Add the ability to configure untagged vlans
>>   bridge: Implement untagged vlan handling
>>   bridge: Dump vlan information from a bridge port
>>   bridge: Add vlan support for local fdb entries
>>
>> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |    5 +-
>> drivers/net/macvlan.c                         |    2 +-
>> drivers/net/vxlan.c                           |    3 +-
>> include/linux/netdevice.h                     |    4 +-
>> include/uapi/linux/if_bridge.h                |   23 ++-
>> include/uapi/linux/neighbour.h                |    1 +
>> include/uapi/linux/rtnetlink.h                |    1 +
>> net/bridge/br_device.c                        |   34 ++-
>> net/bridge/br_fdb.c                           |  253 ++++++++++++---
>> net/bridge/br_forward.c                       |  160 ++++++++++
>> net/bridge/br_if.c                            |  404 ++++++++++++++++++++++++-
>> net/bridge/br_input.c                         |   65 ++++-
>> net/bridge/br_multicast.c                     |   71 +++--
>> net/bridge/br_netlink.c                       |  178 ++++++++++--
>> net/bridge/br_private.h                       |   71 ++++-
>> net/core/rtnetlink.c                          |   40 ++-
>> 16 files changed, 1190 insertions(+), 125 deletions(-)
>>
>> --
>> 1.7.7.6
>>
>> --
>> 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

* [RFC PATCH v3 0/2] Fix some multiqueue TUN problems
From: Paul Moore @ 2012-12-18 22:53 UTC (permalink / raw)
  To: netdev, linux-security-module, selinux; +Cc: jasowang, mst

A refresh/respin of the LSM/SELinux fixes to work on top of Jason's
latest API tweak (now living in DaveM's net tree).  In general, I
believe the hooks and thinking behind the v2 patchset still make sense
so no changes there, although I did change the SELinux permission from
"create_queue" to "attach_queue" to match the API changes.

Comments are welcome and encouraged; we need to get this fixed before
3.8 is released.

---

Paul Moore (2):
      selinux: add the "attach_queue" permission to the "tun_socket" class
      tun: fix LSM/SELinux labeling of tun/tap devices


 drivers/net/tun.c                   |   27 ++++++++++++----
 include/linux/security.h            |   59 +++++++++++++++++++++++++++--------
 security/capability.c               |   24 ++++++++++++--
 security/security.c                 |   28 ++++++++++++++---
 security/selinux/hooks.c            |   50 +++++++++++++++++++++++-------
 security/selinux/include/classmap.h |    2 +
 security/selinux/include/objsec.h   |    4 ++
 7 files changed, 155 insertions(+), 39 deletions(-)

^ permalink raw reply

* [RFC PATCH v3 1/2] selinux: add the "attach_queue" permission to the "tun_socket" class
From: Paul Moore @ 2012-12-18 22:53 UTC (permalink / raw)
  To: netdev, linux-security-module, selinux; +Cc: jasowang, mst
In-Reply-To: <20121218225001.16104.34454.stgit@localhost>

Add a new permission to align with the new TUN multiqueue support,
"tun_socket:attach_queue".

The corresponding SELinux reference policy patch is show below:

 diff --git a/policy/flask/access_vectors b/policy/flask/access_vectors
 index 28802c5..a0664a1 100644
 --- a/policy/flask/access_vectors
 +++ b/policy/flask/access_vectors
 @@ -827,6 +827,9 @@ class kernel_service

  class tun_socket
  inherits socket
 +{
 +       attach_queue
 +}

  class x_pointer
  inherits x_device

Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 security/selinux/include/classmap.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index df2de54..14d04e6 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -150,6 +150,6 @@ struct security_class_mapping secclass_map[] = {
 	    NULL } },
 	{ "kernel_service", { "use_as_override", "create_files_as", NULL } },
 	{ "tun_socket",
-	  { COMMON_SOCK_PERMS, NULL } },
+	  { COMMON_SOCK_PERMS, "attach_queue", NULL } },
 	{ NULL }
   };

^ permalink raw reply related

* [RFC PATCH v3 2/2] tun: fix LSM/SELinux labeling of tun/tap devices
From: Paul Moore @ 2012-12-18 22:53 UTC (permalink / raw)
  To: netdev, linux-security-module, selinux; +Cc: jasowang, mst
In-Reply-To: <20121218225001.16104.34454.stgit@localhost>

This patch corrects some problems with LSM/SELinux that were introduced
with the multiqueue patchset.  The problem stems from the fact that the
multiqueue work changed the relationship between the tun device and its
associated socket; before the socket persisted for the life of the
device, however after the multiqueue changes the socket only persisted
for the life of the userspace connection (fd open).  For non-persistent
devices this is not an issue, but for persistent devices this can cause
the tun device to lose its SELinux label.

We correct this problem by adding an opaque LSM security blob to the
tun device struct which allows us to have the LSM security state, e.g.
SELinux labeling information, persist for the lifetime of the tun
device.  In the process we tweak the LSM hooks to work with this new
approach to TUN device/socket labeling and introduce a new LSM hook,
security_tun_dev_attach_queue(), to approve requests to attach to a
TUN queue via TUNSETQUEUE.

The SELinux code has been adjusted to match the new LSM hooks, the
other LSMs do not make use of the LSM TUN controls.  This patch makes
use of the recently added "tun_socket:attach_queue" permission to
restrict access to the TUNSETQUEUE operation.  On older SELinux
policies which do not define the "tun_socket:attach_queue" permission
the access control decision for TUNSETQUEUE will be handled according
to the SELinux policy's unknown permission setting.

Signed-off-by: Paul Moore <pmoore@redhat.com>
---
 drivers/net/tun.c                 |   27 +++++++++++++----
 include/linux/security.h          |   59 +++++++++++++++++++++++++++++--------
 security/capability.c             |   24 +++++++++++++--
 security/security.c               |   28 ++++++++++++++----
 security/selinux/hooks.c          |   50 ++++++++++++++++++++++++-------
 security/selinux/include/objsec.h |    4 +++
 6 files changed, 154 insertions(+), 38 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 504f7f1..4b7754c 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -186,6 +186,7 @@ struct tun_struct {
 	unsigned long ageing_time;
 	unsigned int numdisabled;
 	struct list_head disabled;
+	void *security;
 };
 
 static inline u32 tun_hashfn(u32 rxhash)
@@ -496,6 +497,10 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
 	struct tun_file *tfile = file->private_data;
 	int err;
 
+	err = security_tun_dev_attach(tfile->socket.sk, tun->security);
+	if (err < 0)
+		goto out;
+
 	err = -EINVAL;
 	if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
 		goto out;
@@ -1389,6 +1394,7 @@ static void tun_free_netdev(struct net_device *dev)
 
 	BUG_ON(!(list_empty(&tun->disabled)));
 	tun_flow_uninit(tun);
+	security_tun_dev_free_security(tun->security);
 	free_netdev(dev);
 }
 
@@ -1575,7 +1581,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		if (tun_not_capable(tun))
 			return -EPERM;
-		err = security_tun_dev_attach(tfile->socket.sk);
+		err = security_tun_dev_open(tun->security);
 		if (err < 0)
 			return err;
 
@@ -1632,7 +1638,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 		spin_lock_init(&tun->lock);
 
-		security_tun_dev_post_create(&tfile->sk);
+		err = security_tun_dev_alloc_security(&tun->security);
+		if (err < 0)
+			goto err_free_dev;
 
 		tun_net_init(dev);
 
@@ -1805,12 +1813,18 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 
 	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
 		tun = tfile->detached;
-		if (!tun)
+		if (!tun) {
 			ret = -EINVAL;
-		else if (tun_not_capable(tun))
+			goto unlock;
+		}
+		if (tun_not_capable(tun)) {
 			ret = -EPERM;
-		else
-			ret = tun_attach(tun, file);
+			goto unlock;
+		}
+		ret = security_tun_dev_attach_queue(tun->security);
+		if (ret < 0)
+			goto unlock;
+		ret = tun_attach(tun, file);
 	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
 		tun = rcu_dereference_protected(tfile->tun,
 						lockdep_rtnl_is_held());
@@ -1821,6 +1835,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 	} else
 		ret = -EINVAL;
 
+unlock:
 	rtnl_unlock();
 	return ret;
 }
diff --git a/include/linux/security.h b/include/linux/security.h
index 05e88bd..e09a87b 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -983,17 +983,29 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	tells the LSM to decrement the number of secmark labeling rules loaded
  * @req_classify_flow:
  *	Sets the flow's sid to the openreq sid.
+ * @tun_dev_alloc_security:
+ *	This hook allows a module to allocate a security structure for a TUN
+ *	device.
+ *	@security pointer to a security structure pointer.
+ *	Returns a zero on success, negative values on failure.
+ * @tun_dev_free_security:
+ *	This hook allows a module to free the security structure for a TUN
+ *	device.
+ *	@security pointer to the TUN device's security structure
  * @tun_dev_create:
  *	Check permissions prior to creating a new TUN device.
- * @tun_dev_post_create:
- *	This hook allows a module to update or allocate a per-socket security
- *	structure.
- *	@sk contains the newly created sock structure.
+ * @tun_dev_attach_queue:
+ *	Check permissions prior to attaching to a TUN device queue.
+ *	@security pointer to the TUN device's security structure.
  * @tun_dev_attach:
- *	Check permissions prior to attaching to a persistent TUN device.  This
- *	hook can also be used by the module to update any security state
+ *	This hook can be used by the module to update any security state
  *	associated with the TUN device's sock structure.
  *	@sk contains the existing sock structure.
+ *	@security pointer to the TUN device's security structure.
+ * @tun_dev_open:
+ *	This hook can be used by the module to update any security state
+ *	associated with the TUN device's security structure.
+ *	@security pointer to the TUN devices's security structure.
  *
  * Security hooks for XFRM operations.
  *
@@ -1613,9 +1625,12 @@ struct security_operations {
 	void (*secmark_refcount_inc) (void);
 	void (*secmark_refcount_dec) (void);
 	void (*req_classify_flow) (const struct request_sock *req, struct flowi *fl);
-	int (*tun_dev_create)(void);
-	void (*tun_dev_post_create)(struct sock *sk);
-	int (*tun_dev_attach)(struct sock *sk);
+	int (*tun_dev_alloc_security) (void **security);
+	void (*tun_dev_free_security) (void *security);
+	int (*tun_dev_create) (void);
+	int (*tun_dev_attach_queue) (void *security);
+	int (*tun_dev_attach) (struct sock *sk, void *security);
+	int (*tun_dev_open) (void *security);
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
@@ -2553,9 +2568,12 @@ void security_inet_conn_established(struct sock *sk,
 int security_secmark_relabel_packet(u32 secid);
 void security_secmark_refcount_inc(void);
 void security_secmark_refcount_dec(void);
+int security_tun_dev_alloc_security(void **security);
+void security_tun_dev_free_security(void *security);
 int security_tun_dev_create(void);
-void security_tun_dev_post_create(struct sock *sk);
-int security_tun_dev_attach(struct sock *sk);
+int security_tun_dev_attach_queue(void *security);
+int security_tun_dev_attach(struct sock *sk, void *security);
+int security_tun_dev_open(void *security);
 
 #else	/* CONFIG_SECURITY_NETWORK */
 static inline int security_unix_stream_connect(struct sock *sock,
@@ -2720,16 +2738,31 @@ static inline void security_secmark_refcount_dec(void)
 {
 }
 
+static inline int security_tun_dev_alloc_security(void **security)
+{
+	return 0;
+}
+
+static inline void security_tun_dev_free_security(void *security)
+{
+}
+
 static inline int security_tun_dev_create(void)
 {
 	return 0;
 }
 
-static inline void security_tun_dev_post_create(struct sock *sk)
+static inline int security_tun_dev_attach_queue(void *security)
+{
+	return 0;
+}
+
+static inline int security_tun_dev_attach(struct sock *sk, void *security)
 {
+	return 0;
 }
 
-static inline int security_tun_dev_attach(struct sock *sk)
+static inline int security_tun_dev_open(void *security)
 {
 	return 0;
 }
diff --git a/security/capability.c b/security/capability.c
index b14a30c..76c1dc9 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -704,16 +704,31 @@ static void cap_req_classify_flow(const struct request_sock *req,
 {
 }
 
+static int cap_tun_dev_alloc_security(void **security)
+{
+	return 0;
+}
+
+static void cap_tun_dev_free_security(void *security)
+{
+}
+
 static int cap_tun_dev_create(void)
 {
 	return 0;
 }
 
-static void cap_tun_dev_post_create(struct sock *sk)
+static int cap_tun_dev_attach_queue(void *security)
+{
+	return 0;
+}
+
+static int cap_tun_dev_attach(struct sock *sk, void *security)
 {
+	return 0;
 }
 
-static int cap_tun_dev_attach(struct sock *sk)
+static int cap_tun_dev_open(void *security)
 {
 	return 0;
 }
@@ -1044,8 +1059,11 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, secmark_refcount_inc);
 	set_to_cap_if_null(ops, secmark_refcount_dec);
 	set_to_cap_if_null(ops, req_classify_flow);
+	set_to_cap_if_null(ops, tun_dev_alloc_security);
+	set_to_cap_if_null(ops, tun_dev_free_security);
 	set_to_cap_if_null(ops, tun_dev_create);
-	set_to_cap_if_null(ops, tun_dev_post_create);
+	set_to_cap_if_null(ops, tun_dev_open);
+	set_to_cap_if_null(ops, tun_dev_attach_queue);
 	set_to_cap_if_null(ops, tun_dev_attach);
 #endif	/* CONFIG_SECURITY_NETWORK */
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
diff --git a/security/security.c b/security/security.c
index 8dcd4ae..a271ed4 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1244,24 +1244,42 @@ void security_secmark_refcount_dec(void)
 }
 EXPORT_SYMBOL(security_secmark_refcount_dec);
 
+int security_tun_dev_alloc_security(void **security)
+{
+	return security_ops->tun_dev_alloc_security(security);
+}
+EXPORT_SYMBOL(security_tun_dev_alloc_security);
+
+void security_tun_dev_free_security(void *security)
+{
+	security_ops->tun_dev_free_security(security);
+}
+EXPORT_SYMBOL(security_tun_dev_free_security);
+
 int security_tun_dev_create(void)
 {
 	return security_ops->tun_dev_create();
 }
 EXPORT_SYMBOL(security_tun_dev_create);
 
-void security_tun_dev_post_create(struct sock *sk)
+int security_tun_dev_attach_queue(void *security)
 {
-	return security_ops->tun_dev_post_create(sk);
+	return security_ops->tun_dev_attach_queue(security);
 }
-EXPORT_SYMBOL(security_tun_dev_post_create);
+EXPORT_SYMBOL(security_tun_dev_attach_queue);
 
-int security_tun_dev_attach(struct sock *sk)
+int security_tun_dev_attach(struct sock *sk, void *security)
 {
-	return security_ops->tun_dev_attach(sk);
+	return security_ops->tun_dev_attach(sk, security);
 }
 EXPORT_SYMBOL(security_tun_dev_attach);
 
+int security_tun_dev_open(void *security)
+{
+	return security_ops->tun_dev_open(security);
+}
+EXPORT_SYMBOL(security_tun_dev_open);
+
 #endif	/* CONFIG_SECURITY_NETWORK */
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 61a5336..ef26e96 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4399,6 +4399,24 @@ static void selinux_req_classify_flow(const struct request_sock *req,
 	fl->flowi_secid = req->secid;
 }
 
+static int selinux_tun_dev_alloc_security(void **security)
+{
+	struct tun_security_struct *tunsec;
+
+	tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
+	if (!tunsec)
+		return -ENOMEM;
+	tunsec->sid = current_sid();
+
+	*security = tunsec;
+	return 0;
+}
+
+static void selinux_tun_dev_free_security(void *security)
+{
+	kfree(security);
+}
+
 static int selinux_tun_dev_create(void)
 {
 	u32 sid = current_sid();
@@ -4414,8 +4432,17 @@ static int selinux_tun_dev_create(void)
 			    NULL);
 }
 
-static void selinux_tun_dev_post_create(struct sock *sk)
+static int selinux_tun_dev_attach_queue(void *security)
 {
+	struct tun_security_struct *tunsec = security;
+
+	return avc_has_perm(current_sid(), tunsec->sid, SECCLASS_TUN_SOCKET,
+			    TUN_SOCKET__ATTACH_QUEUE, NULL);
+}
+
+static int selinux_tun_dev_attach(struct sock *sk, void *security)
+{
+	struct tun_security_struct *tunsec = security;
 	struct sk_security_struct *sksec = sk->sk_security;
 
 	/* we don't currently perform any NetLabel based labeling here and it
@@ -4425,20 +4452,19 @@ static void selinux_tun_dev_post_create(struct sock *sk)
 	 * cause confusion to the TUN user that had no idea network labeling
 	 * protocols were being used */
 
-	/* see the comments in selinux_tun_dev_create() about why we don't use
-	 * the sockcreate SID here */
-
-	sksec->sid = current_sid();
+	sksec->sid = tunsec->sid;
 	sksec->sclass = SECCLASS_TUN_SOCKET;
+
+	return 0;
 }
 
-static int selinux_tun_dev_attach(struct sock *sk)
+static int selinux_tun_dev_open(void *security)
 {
-	struct sk_security_struct *sksec = sk->sk_security;
+	struct tun_security_struct *tunsec = security;
 	u32 sid = current_sid();
 	int err;
 
-	err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET,
+	err = avc_has_perm(sid, tunsec->sid, SECCLASS_TUN_SOCKET,
 			   TUN_SOCKET__RELABELFROM, NULL);
 	if (err)
 		return err;
@@ -4446,8 +4472,7 @@ static int selinux_tun_dev_attach(struct sock *sk)
 			   TUN_SOCKET__RELABELTO, NULL);
 	if (err)
 		return err;
-
-	sksec->sid = sid;
+	tunsec->sid = sid;
 
 	return 0;
 }
@@ -5642,9 +5667,12 @@ static struct security_operations selinux_ops = {
 	.secmark_refcount_inc =		selinux_secmark_refcount_inc,
 	.secmark_refcount_dec =		selinux_secmark_refcount_dec,
 	.req_classify_flow =		selinux_req_classify_flow,
+	.tun_dev_alloc_security =	selinux_tun_dev_alloc_security,
+	.tun_dev_free_security =	selinux_tun_dev_free_security,
 	.tun_dev_create =		selinux_tun_dev_create,
-	.tun_dev_post_create = 		selinux_tun_dev_post_create,
+	.tun_dev_attach_queue =		selinux_tun_dev_attach_queue,
 	.tun_dev_attach =		selinux_tun_dev_attach,
+	.tun_dev_open =			selinux_tun_dev_open,
 
 #ifdef CONFIG_SECURITY_NETWORK_XFRM
 	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
index 26c7eee..aa47bca 100644
--- a/security/selinux/include/objsec.h
+++ b/security/selinux/include/objsec.h
@@ -110,6 +110,10 @@ struct sk_security_struct {
 	u16 sclass;			/* sock security class */
 };
 
+struct tun_security_struct {
+	u32 sid;			/* SID for the tun device sockets */
+};
+
 struct key_security_struct {
 	u32 sid;	/* SID of key */
 };

^ permalink raw reply related

* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
From: Michael S. Tsirkin @ 2012-12-18 23:01 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
In-Reply-To: <1355857263-31197-10-git-send-email-vyasevic@redhat.com>

On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> A user may designate a certain vlan as untagged.  This means that
> any ingress frame is assigned to this vlan and any forwarding decisions
> are made with this vlan in mind.  On egress, any frames tagged/labeled
> with untagged vlan have the vlan tag removed and are send as regular
> ethernet frames.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  include/uapi/linux/if_bridge.h |    3 +
>  net/bridge/br_if.c             |  146 +++++++++++++++++++++++++++++++++++++---
>  net/bridge/br_netlink.c        |    6 +-
>  net/bridge/br_private.h        |    2 +
>  4 files changed, 144 insertions(+), 13 deletions(-)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index d0b4f5c..988d858 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -127,6 +127,9 @@ enum {
>  	BR_VLAN_DEL,
>  };
>  
> +#define BRIDGE_VLAN_INFO_MASTER		1
> +#define BRIDGE_VLAN_INFO_UNTAGGED	2
> +
>  struct bridge_vlan_info {
>  	u16 op_code;
>  	u16 flags;
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 57bbb35..14563fb 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>  		br_vlan_destroy(vlan);
>  }
>  
> +/* Must be protected by RTNL */
> +static void br_vlan_add_untagged(struct net_bridge *br,
> +				 struct net_bridge_vlan *vlan)
> +{
> +	ASSERT_RTNL();
> +	if (br->untagged == vlan)
> +		return;
> +	else if (br->untagged) {
> +		/* Untagged vlan is already set on the master,
> +		 * so drop the ref since we'll be replacing it.
> +		 */
> +		br_vlan_put(br->untagged);
> +	}
> +	br_vlan_hold(vlan);
> +	rcu_assign_pointer(br->untagged, vlan);

Is there a reason for rcu here but not else where? If all users are under
rtnl you can just assign in a simple way.
If not then rcu_dereference_protected would be more appropriate.

> +}
> +
> +/* Must be protected by RTNL */
> +static void br_vlan_del_untagged(struct net_bridge *br,
> +				 struct net_bridge_vlan *vlan)
> +{
> +	ASSERT_RTNL();
> +	if (br->untagged == vlan) {
> +		br_vlan_put(vlan);
> +		rcu_assign_pointer(br->untagged, NULL);
> +	}
> +}
> +
>  struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>  {
>  	struct net_bridge_vlan *vlan;
> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  
>  	vlan = br_vlan_find(br, vid);
>  	if (vlan)
> -		return vlan;
> +		goto untagged;
>  
>  	vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>  	if (!vlan)
> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  	vlan->vid = vid;
>  	atomic_set(&vlan->refcnt, 1);
>  
> -	if (flags & BRIDGE_FLAGS_SELF) {
> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>  		/* Set bit 0 that is associated with the bridge master
>  		 * device.  Port numbers start with 1.
>  		 */
> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  	}
>  
>  	hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> +
> +untagged:
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		br_vlan_add_untagged(br, vlan);
> +
>  	return vlan;
>  }
>  
>  /* Must be protected by RTNL */
> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> +			u16 flags)
>  {
>  	ASSERT_RTNL();
>  
> -	if (flags & BRIDGE_FLAGS_SELF) {
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		br_vlan_del_untagged(br, vlan);
> +
> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>  		/* Clear bit 0 that is associated with the bridge master
>  		 * device.
>  		 */
> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>  
>  	vlan->vid = BR_INVALID_VID;
>  
> +	/* If, for whatever reason, bridge still has a ref on this vlan
> +	 * through the @untagged pointer, drop that ref and clear untagged.
> +	 */
> +	if (br->untagged == vlan) {
> +		br_vlan_put(vlan);
> +		rcu_assign_pointer(br->untagged, NULL);
> +	}
> +
>  	/* Drop the self-ref to trigger descrution. */
>  	br_vlan_put(vlan);
>  }
> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>  	if (!vlan)
>  		return -ENOENT;
>  
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(br, vlan, flags);
>  	return 0;
>  }
>  
> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>  	for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>  		hlist_for_each_entry_safe(vlan, node, tmp,
>  					  &br->vlan_hlist[i], hlist) {
> -			br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> +			br_vlan_del(br, vlan,
> +				    (BRIDGE_VLAN_INFO_MASTER |
> +				     BRIDGE_VLAN_INFO_UNTAGGED));
>  		}
>  	}
>  }
> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>  	return NULL;
>  }
>  
> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> +			  struct net_bridge_vlan *vlan,
> +			  u16 flags)
> +{
> +	struct net_device *dev = p->dev;
> +
> +	if (p->untagged) {
> +		/* Port already has untagged vlan set.  Drop the ref
> +		 * to the old one since we'll be replace it.
> +		 */
> +		br_vlan_put(p->untagged);
> +	} else {
> +		int err;
> +
> +		/* Add vid 0 to filter if filter is available. */
> +		if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +		    dev->netdev_ops->ndo_vlan_rx_add_vid &&
> +		    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> +			err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	/* This VLAN is handled as untagged/native. Save an
> +	 * additional ref.
> +	 */
> +	br_vlan_hold(vlan);
> +	rcu_assign_pointer(p->untagged, vlan);
> +
> +	return 0;
> +}
> +
> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> +				     struct net_bridge_vlan *vlan)
> +{
> +	if (p->untagged != vlan)
> +		return;
> +
> +	/* Remove VLAN from the device filter if it is supported. */
> +	if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> +		int err;
> +
> +		err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> +		if (err) {
> +			pr_warn("failed to kill vid %d for device %s\n",
> +				vlan->vid, p->dev->name);
> +		}
> +	}
> +
> +	/* If this VLAN is currently functioning as untagged, clear it.
> +	 * It's safe to drop the refcount, since the vlan is still held
> +	 * by the port.
> +	 */
> +	br_vlan_put(vlan);
> +	rcu_assign_pointer(p->untagged, NULL);
> +
> +}
> +
>  /* Must be protected by RTNL */
>  int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>  {
> -	struct net_port_vlan *pve;
> +	struct net_port_vlan *pve = NULL;
>  	struct net_bridge_vlan *vlan;
>  	struct net_device *dev = p->dev;
>  	int err;
> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>  	set_bit(p->port_no, vlan->port_bitmap);
>  
>  	list_add_tail_rcu(&pve->list, &p->vlan_list);
> +
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> +		err = nbp_vlan_add_untagged(p, vlan, flags);
> +		if (err)
> +			goto del_vlan;
> +	}
> +
>  	return 0;
>  
>  clean_up:
>  	kfree(pve);
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(p->br, vlan, flags);
> +	return err;
> +del_vlan:
> +	nbp_vlan_delete(p, vid, flags);
>  	return err;
>  }
>  
> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  	if (!pve)
>  		return -ENOENT;
>  
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		nbp_vlan_delete_untagged(p, pve->vlan);
> +
>  	/* Remove VLAN from the device filter if it is supported. */
>  	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>  	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  			pr_warn("failed to kill vid %d for device %s\n",
>  				vid, dev->name);
>  	}
> +
>  	pve->vid = BR_INVALID_VID;
>  
>  	vlan = pve->vlan;
> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  	list_del_rcu(&pve->list);
>  	kfree_rcu(pve, rcu);
>  
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(p->br, vlan, flags);
>  
>  	return 0;
>  }
> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>  
>  	ASSERT_RTNL();
>  
> -	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> -		nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> +	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)  {
> +		nbp_vlan_delete(p, pve->vid,
> +				(BRIDGE_VLAN_INFO_MASTER |
> +				 BRIDGE_VLAN_INFO_UNTAGGED));
> +	}
>  }
>  
>  static void release_nbp(struct kobject *kobj)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9cf2879..1b302ce 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>  			if (p)
>  				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>  			else {
> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> +				u16 flags = vinfo->flags |
> +					    BRIDGE_VLAN_INFO_MASTER;
>  				if (!br_vlan_add(br, vinfo->vid, flags))
>  					err = -ENOMEM;
>  			}
> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>  				err = nbp_vlan_delete(p, vinfo->vid,
>  						      vinfo->flags);
>  			else {
> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> +				u16 flags = vinfo->flags |
> +					    BRIDGE_VLAN_INFO_MASTER;
>  				err = br_vlan_delete(br, vinfo->vid, flags);
>  			}
>  			break;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index cc75212..9328463 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -179,6 +179,7 @@ struct net_bridge_port
>  	struct netpoll			*np;
>  #endif
>  	struct list_head		vlan_list;
> +	struct net_bridge_vlan __rcu	*untagged;
>  };
>  
>  #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> @@ -298,6 +299,7 @@ struct net_bridge
>  	struct timer_list		gc_timer;
>  	struct kobject			*ifobj;
>  	struct hlist_head		vlan_hlist[BR_VID_HASH_SIZE];
> +	struct net_bridge_vlan __rcu	*untagged;
>  };
>  
>  struct br_input_skb_cb {
> -- 
> 1.7.7.6

^ permalink raw reply

* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
From: Michael S. Tsirkin @ 2012-12-18 23:04 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
In-Reply-To: <1355857263-31197-10-git-send-email-vyasevic@redhat.com>

On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> A user may designate a certain vlan as untagged.  This means that
> any ingress frame is assigned to this vlan and any forwarding decisions
> are made with this vlan in mind.  On egress, any frames tagged/labeled
> with untagged vlan have the vlan tag removed and are send as regular
> ethernet frames.
> 
> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> ---
>  include/uapi/linux/if_bridge.h |    3 +
>  net/bridge/br_if.c             |  146 +++++++++++++++++++++++++++++++++++++---
>  net/bridge/br_netlink.c        |    6 +-
>  net/bridge/br_private.h        |    2 +
>  4 files changed, 144 insertions(+), 13 deletions(-)
> 
> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> index d0b4f5c..988d858 100644
> --- a/include/uapi/linux/if_bridge.h
> +++ b/include/uapi/linux/if_bridge.h
> @@ -127,6 +127,9 @@ enum {
>  	BR_VLAN_DEL,
>  };
>  
> +#define BRIDGE_VLAN_INFO_MASTER		1
> +#define BRIDGE_VLAN_INFO_UNTAGGED	2
> +
>  struct bridge_vlan_info {
>  	u16 op_code;
>  	u16 flags;
> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> index 57bbb35..14563fb 100644
> --- a/net/bridge/br_if.c
> +++ b/net/bridge/br_if.c
> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>  		br_vlan_destroy(vlan);
>  }
>  
> +/* Must be protected by RTNL */
> +static void br_vlan_add_untagged(struct net_bridge *br,
> +				 struct net_bridge_vlan *vlan)
> +{
> +	ASSERT_RTNL();
> +	if (br->untagged == vlan)
> +		return;
> +	else if (br->untagged) {
> +		/* Untagged vlan is already set on the master,
> +		 * so drop the ref since we'll be replacing it.
> +		 */
> +		br_vlan_put(br->untagged);
> +	}
> +	br_vlan_hold(vlan);
> +	rcu_assign_pointer(br->untagged, vlan);
> +}
> +
> +/* Must be protected by RTNL */
> +static void br_vlan_del_untagged(struct net_bridge *br,
> +				 struct net_bridge_vlan *vlan)
> +{
> +	ASSERT_RTNL();
> +	if (br->untagged == vlan) {
> +		br_vlan_put(vlan);
> +		rcu_assign_pointer(br->untagged, NULL);
> +	}
> +}
> +
>  struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>  {
>  	struct net_bridge_vlan *vlan;
> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  
>  	vlan = br_vlan_find(br, vid);
>  	if (vlan)
> -		return vlan;
> +		goto untagged;
>  
>  	vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>  	if (!vlan)
> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  	vlan->vid = vid;
>  	atomic_set(&vlan->refcnt, 1);
>  
> -	if (flags & BRIDGE_FLAGS_SELF) {
> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>  		/* Set bit 0 that is associated with the bridge master
>  		 * device.  Port numbers start with 1.
>  		 */
> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>  	}
>  
>  	hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> +
> +untagged:
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		br_vlan_add_untagged(br, vlan);
> +
>  	return vlan;
>  }
>  
>  /* Must be protected by RTNL */
> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> +			u16 flags)
>  {
>  	ASSERT_RTNL();
>  
> -	if (flags & BRIDGE_FLAGS_SELF) {
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		br_vlan_del_untagged(br, vlan);
> +
> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>  		/* Clear bit 0 that is associated with the bridge master
>  		 * device.
>  		 */
> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>  
>  	vlan->vid = BR_INVALID_VID;
>  
> +	/* If, for whatever reason, bridge still has a ref on this vlan
> +	 * through the @untagged pointer, drop that ref and clear untagged.
> +	 */
> +	if (br->untagged == vlan) {
> +		br_vlan_put(vlan);
> +		rcu_assign_pointer(br->untagged, NULL);

Is something doing an rcu sync after this point?
If yes maybe add a comment saying where it is, if not - some rcu read
side could still be using it through this pointer.

> +	}
> +
>  	/* Drop the self-ref to trigger descrution. */
>  	br_vlan_put(vlan);
>  }
> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>  	if (!vlan)
>  		return -ENOENT;
>  
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(br, vlan, flags);
>  	return 0;
>  }
>  
> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>  	for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>  		hlist_for_each_entry_safe(vlan, node, tmp,
>  					  &br->vlan_hlist[i], hlist) {
> -			br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> +			br_vlan_del(br, vlan,
> +				    (BRIDGE_VLAN_INFO_MASTER |
> +				     BRIDGE_VLAN_INFO_UNTAGGED));
>  		}
>  	}
>  }
> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>  	return NULL;
>  }
>  
> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> +			  struct net_bridge_vlan *vlan,
> +			  u16 flags)
> +{
> +	struct net_device *dev = p->dev;
> +
> +	if (p->untagged) {
> +		/* Port already has untagged vlan set.  Drop the ref
> +		 * to the old one since we'll be replace it.
> +		 */
> +		br_vlan_put(p->untagged);
> +	} else {
> +		int err;
> +
> +		/* Add vid 0 to filter if filter is available. */
> +		if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +		    dev->netdev_ops->ndo_vlan_rx_add_vid &&
> +		    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> +			err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	/* This VLAN is handled as untagged/native. Save an
> +	 * additional ref.
> +	 */
> +	br_vlan_hold(vlan);
> +	rcu_assign_pointer(p->untagged, vlan);
> +
> +	return 0;
> +}
> +
> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> +				     struct net_bridge_vlan *vlan)
> +{
> +	if (p->untagged != vlan)
> +		return;
> +
> +	/* Remove VLAN from the device filter if it is supported. */
> +	if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> +	    p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> +		int err;
> +
> +		err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> +		if (err) {
> +			pr_warn("failed to kill vid %d for device %s\n",
> +				vlan->vid, p->dev->name);
> +		}
> +	}
> +
> +	/* If this VLAN is currently functioning as untagged, clear it.
> +	 * It's safe to drop the refcount, since the vlan is still held
> +	 * by the port.
> +	 */
> +	br_vlan_put(vlan);
> +	rcu_assign_pointer(p->untagged, NULL);
> +
> +}
> +
>  /* Must be protected by RTNL */
>  int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>  {
> -	struct net_port_vlan *pve;
> +	struct net_port_vlan *pve = NULL;
>  	struct net_bridge_vlan *vlan;
>  	struct net_device *dev = p->dev;
>  	int err;
> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>  	set_bit(p->port_no, vlan->port_bitmap);
>  
>  	list_add_tail_rcu(&pve->list, &p->vlan_list);
> +
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> +		err = nbp_vlan_add_untagged(p, vlan, flags);
> +		if (err)
> +			goto del_vlan;
> +	}
> +
>  	return 0;
>  
>  clean_up:
>  	kfree(pve);
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(p->br, vlan, flags);
> +	return err;
> +del_vlan:
> +	nbp_vlan_delete(p, vid, flags);
>  	return err;
>  }
>  
> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  	if (!pve)
>  		return -ENOENT;
>  
> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> +		nbp_vlan_delete_untagged(p, pve->vlan);
> +
>  	/* Remove VLAN from the device filter if it is supported. */
>  	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>  	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  			pr_warn("failed to kill vid %d for device %s\n",
>  				vid, dev->name);
>  	}
> +
>  	pve->vid = BR_INVALID_VID;
>  
>  	vlan = pve->vlan;
> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>  	list_del_rcu(&pve->list);
>  	kfree_rcu(pve, rcu);
>  
> -	br_vlan_del(vlan, flags);
> +	br_vlan_del(p->br, vlan, flags);
>  
>  	return 0;
>  }
> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>  
>  	ASSERT_RTNL();
>  
> -	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> -		nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> +	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)  {
> +		nbp_vlan_delete(p, pve->vid,
> +				(BRIDGE_VLAN_INFO_MASTER |
> +				 BRIDGE_VLAN_INFO_UNTAGGED));
> +	}
>  }
>  
>  static void release_nbp(struct kobject *kobj)
> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> index 9cf2879..1b302ce 100644
> --- a/net/bridge/br_netlink.c
> +++ b/net/bridge/br_netlink.c
> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>  			if (p)
>  				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>  			else {
> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> +				u16 flags = vinfo->flags |
> +					    BRIDGE_VLAN_INFO_MASTER;
>  				if (!br_vlan_add(br, vinfo->vid, flags))
>  					err = -ENOMEM;
>  			}
> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>  				err = nbp_vlan_delete(p, vinfo->vid,
>  						      vinfo->flags);
>  			else {
> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> +				u16 flags = vinfo->flags |
> +					    BRIDGE_VLAN_INFO_MASTER;
>  				err = br_vlan_delete(br, vinfo->vid, flags);
>  			}
>  			break;
> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> index cc75212..9328463 100644
> --- a/net/bridge/br_private.h
> +++ b/net/bridge/br_private.h
> @@ -179,6 +179,7 @@ struct net_bridge_port
>  	struct netpoll			*np;
>  #endif
>  	struct list_head		vlan_list;
> +	struct net_bridge_vlan __rcu	*untagged;
>  };
>  
>  #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> @@ -298,6 +299,7 @@ struct net_bridge
>  	struct timer_list		gc_timer;
>  	struct kobject			*ifobj;
>  	struct hlist_head		vlan_hlist[BR_VID_HASH_SIZE];
> +	struct net_bridge_vlan __rcu	*untagged;
>  };
>  
>  struct br_input_skb_cb {
> -- 
> 1.7.7.6

^ permalink raw reply

* Re: [patch net-next 1/4] net: add change_carrier netdev op
From: Dan Williams @ 2012-12-18 23:02 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, edumazet, bhutchings, mirqus, shemminger, greearb,
	fbl, john.r.fastabend
In-Reply-To: <1355868858-1713-2-git-send-email-jiri@resnulli.us>

On Tue, 2012-12-18 at 23:14 +0100, Jiri Pirko wrote:
> This allows a driver to register change_carrier callback which will be
> called whenever user will like to change carrier state. This is useful
> for devices like dummy, gre, team and so on.
> 
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
>  include/linux/netdevice.h |  9 +++++++++
>  net/core/dev.c            | 19 +++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 02e0f6b..8047330 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -891,6 +891,11 @@ struct netdev_fcoe_hbainfo {
>   * int (*ndo_bridge_setlink)(struct net_device *dev, struct nlmsghdr *nlh)
>   * int (*ndo_bridge_getlink)(struct sk_buff *skb, u32 pid, u32 seq,
>   *			     struct net_device *dev)
> + *
> + * int (*ndo_change_carrier)(struct net_device *dev, bool new_carrier);
> + *	Called to update device carrier. Soft-devices which do not manage
> + *	real hardware like dummy, team, etc. can define this to provide
> + *	possibility to set their carrier state.

How about something like:

---
Called to change device carrier.  Virtual devices (like dummy, team,
etc) which do not represent real hardware may define this to allow their
userspace components to manage their virtual carrier state.  Devices
that determine carrier state from physical hardware properties (eg
network cables) or protocol-dependent mechanisms (eg
USB_CDC_NOTIFY_NETWORK_CONNECTION) should NOT implement this function.
---

I'm just worried that without some clearer wording, somebody will end up
implementing the function for an actual hardware driver down the road
and expect things to work when they change the carrier to "on" but the
protocol isn't set up or cable isn't plugged in.  I'm also worried that
it might be used to simply work around bugs in the drivers that should
be fixed in the drivers instead.

Dan

>  struct net_device_ops {
>  	int			(*ndo_init)(struct net_device *dev);
> @@ -1008,6 +1013,8 @@ struct net_device_ops {
>  	int			(*ndo_bridge_getlink)(struct sk_buff *skb,
>  						      u32 pid, u32 seq,
>  						      struct net_device *dev);
> +	int			(*ndo_change_carrier)(struct net_device *dev,
> +						      bool new_carrier);
>  };
>  
>  /*
> @@ -2194,6 +2201,8 @@ extern int		dev_set_mtu(struct net_device *, int);
>  extern void		dev_set_group(struct net_device *, int);
>  extern int		dev_set_mac_address(struct net_device *,
>  					    struct sockaddr *);
> +extern int		dev_change_carrier(struct net_device *,
> +					   bool new_carrier);
>  extern int		dev_hard_start_xmit(struct sk_buff *skb,
>  					    struct net_device *dev,
>  					    struct netdev_queue *txq);
> diff --git a/net/core/dev.c b/net/core/dev.c
> index d0cbc93..268a714 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5027,6 +5027,25 @@ int dev_set_mac_address(struct net_device *dev, struct sockaddr *sa)
>  }
>  EXPORT_SYMBOL(dev_set_mac_address);
>  
> +/**
> + *	dev_change_carrier - Change device carrier
> + *	@dev: device
> + *	@new_carries: new value
> + *
> + *	Change device carrier
> + */
> +int dev_change_carrier(struct net_device *dev, bool new_carrier)
> +{
> +	const struct net_device_ops *ops = dev->netdev_ops;
> +
> +	if (!ops->ndo_change_carrier)
> +		return -EOPNOTSUPP;
> +	if (!netif_device_present(dev))
> +		return -ENODEV;
> +	return ops->ndo_change_carrier(dev, new_carrier);
> +}
> +EXPORT_SYMBOL(dev_change_carrier);
> +
>  /*
>   *	Perform the SIOCxIFxxx calls, inside rcu_read_lock()
>   */

^ permalink raw reply

* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
From: Vlad Yasevich @ 2012-12-18 23:03 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
In-Reply-To: <20121218230107.GA1135@redhat.com>

On 12/18/2012 06:01 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
>> A user may designate a certain vlan as untagged.  This means that
>> any ingress frame is assigned to this vlan and any forwarding decisions
>> are made with this vlan in mind.  On egress, any frames tagged/labeled
>> with untagged vlan have the vlan tag removed and are send as regular
>> ethernet frames.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   include/uapi/linux/if_bridge.h |    3 +
>>   net/bridge/br_if.c             |  146 +++++++++++++++++++++++++++++++++++++---
>>   net/bridge/br_netlink.c        |    6 +-
>>   net/bridge/br_private.h        |    2 +
>>   4 files changed, 144 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index d0b4f5c..988d858 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -127,6 +127,9 @@ enum {
>>   	BR_VLAN_DEL,
>>   };
>>
>> +#define BRIDGE_VLAN_INFO_MASTER		1
>> +#define BRIDGE_VLAN_INFO_UNTAGGED	2
>> +
>>   struct bridge_vlan_info {
>>   	u16 op_code;
>>   	u16 flags;
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 57bbb35..14563fb 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>>   		br_vlan_destroy(vlan);
>>   }
>>
>> +/* Must be protected by RTNL */
>> +static void br_vlan_add_untagged(struct net_bridge *br,
>> +				 struct net_bridge_vlan *vlan)
>> +{
>> +	ASSERT_RTNL();
>> +	if (br->untagged == vlan)
>> +		return;
>> +	else if (br->untagged) {
>> +		/* Untagged vlan is already set on the master,
>> +		 * so drop the ref since we'll be replacing it.
>> +		 */
>> +		br_vlan_put(br->untagged);
>> +	}
>> +	br_vlan_hold(vlan);
>> +	rcu_assign_pointer(br->untagged, vlan);
>
> Is there a reason for rcu here but not else where? If all users are under
> rtnl you can just assign in a simple way.
> If not then rcu_dereference_protected would be more appropriate.

Everywhere that the pointer changes rcu_assign_pointer is used.

Now, if we hold an RTNL, we can technically read the pointer with rcu 
since it's guaranteed not to change since it only changes under RTNL.
I'll check that this is consistent.

If I access the pointer without rtnl, it's always inside rcu critical 
section and with rcu_dereference().

I thought those were the basic rules of rcu.  Did that change?

-vlad

>
>> +}
>> +
>> +/* Must be protected by RTNL */
>> +static void br_vlan_del_untagged(struct net_bridge *br,
>> +				 struct net_bridge_vlan *vlan)
>> +{
>> +	ASSERT_RTNL();
>> +	if (br->untagged == vlan) {
>> +		br_vlan_put(vlan);
>> +		rcu_assign_pointer(br->untagged, NULL);
>> +	}
>> +}
>> +
>>   struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>>   {
>>   	struct net_bridge_vlan *vlan;
>> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>
>>   	vlan = br_vlan_find(br, vid);
>>   	if (vlan)
>> -		return vlan;
>> +		goto untagged;
>>
>>   	vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>>   	if (!vlan)
>> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>   	vlan->vid = vid;
>>   	atomic_set(&vlan->refcnt, 1);
>>
>> -	if (flags & BRIDGE_FLAGS_SELF) {
>> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>   		/* Set bit 0 that is associated with the bridge master
>>   		 * device.  Port numbers start with 1.
>>   		 */
>> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>   	}
>>
>>   	hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
>> +
>> +untagged:
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		br_vlan_add_untagged(br, vlan);
>> +
>>   	return vlan;
>>   }
>>
>>   /* Must be protected by RTNL */
>> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
>> +			u16 flags)
>>   {
>>   	ASSERT_RTNL();
>>
>> -	if (flags & BRIDGE_FLAGS_SELF) {
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		br_vlan_del_untagged(br, vlan);
>> +
>> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>   		/* Clear bit 0 that is associated with the bridge master
>>   		 * device.
>>   		 */
>> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>
>>   	vlan->vid = BR_INVALID_VID;
>>
>> +	/* If, for whatever reason, bridge still has a ref on this vlan
>> +	 * through the @untagged pointer, drop that ref and clear untagged.
>> +	 */
>> +	if (br->untagged == vlan) {
>> +		br_vlan_put(vlan);
>> +		rcu_assign_pointer(br->untagged, NULL);
>> +	}
>> +
>>   	/* Drop the self-ref to trigger descrution. */
>>   	br_vlan_put(vlan);
>>   }
>> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>>   	if (!vlan)
>>   		return -ENOENT;
>>
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(br, vlan, flags);
>>   	return 0;
>>   }
>>
>> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>>   	for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>>   		hlist_for_each_entry_safe(vlan, node, tmp,
>>   					  &br->vlan_hlist[i], hlist) {
>> -			br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
>> +			br_vlan_del(br, vlan,
>> +				    (BRIDGE_VLAN_INFO_MASTER |
>> +				     BRIDGE_VLAN_INFO_UNTAGGED));
>>   		}
>>   	}
>>   }
>> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>>   	return NULL;
>>   }
>>
>> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
>> +			  struct net_bridge_vlan *vlan,
>> +			  u16 flags)
>> +{
>> +	struct net_device *dev = p->dev;
>> +
>> +	if (p->untagged) {
>> +		/* Port already has untagged vlan set.  Drop the ref
>> +		 * to the old one since we'll be replace it.
>> +		 */
>> +		br_vlan_put(p->untagged);
>> +	} else {
>> +		int err;
>> +
>> +		/* Add vid 0 to filter if filter is available. */
>> +		if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +		    dev->netdev_ops->ndo_vlan_rx_add_vid &&
>> +		    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> +			err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
>> +			if (err)
>> +				return err;
>> +		}
>> +	}
>> +
>> +	/* This VLAN is handled as untagged/native. Save an
>> +	 * additional ref.
>> +	 */
>> +	br_vlan_hold(vlan);
>> +	rcu_assign_pointer(p->untagged, vlan);
>> +
>> +	return 0;
>> +}
>> +
>> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
>> +				     struct net_bridge_vlan *vlan)
>> +{
>> +	if (p->untagged != vlan)
>> +		return;
>> +
>> +	/* Remove VLAN from the device filter if it is supported. */
>> +	if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +	    p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> +		int err;
>> +
>> +		err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
>> +		if (err) {
>> +			pr_warn("failed to kill vid %d for device %s\n",
>> +				vlan->vid, p->dev->name);
>> +		}
>> +	}
>> +
>> +	/* If this VLAN is currently functioning as untagged, clear it.
>> +	 * It's safe to drop the refcount, since the vlan is still held
>> +	 * by the port.
>> +	 */
>> +	br_vlan_put(vlan);
>> +	rcu_assign_pointer(p->untagged, NULL);
>> +
>> +}
>> +
>>   /* Must be protected by RTNL */
>>   int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>   {
>> -	struct net_port_vlan *pve;
>> +	struct net_port_vlan *pve = NULL;
>>   	struct net_bridge_vlan *vlan;
>>   	struct net_device *dev = p->dev;
>>   	int err;
>> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	set_bit(p->port_no, vlan->port_bitmap);
>>
>>   	list_add_tail_rcu(&pve->list, &p->vlan_list);
>> +
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
>> +		err = nbp_vlan_add_untagged(p, vlan, flags);
>> +		if (err)
>> +			goto del_vlan;
>> +	}
>> +
>>   	return 0;
>>
>>   clean_up:
>>   	kfree(pve);
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(p->br, vlan, flags);
>> +	return err;
>> +del_vlan:
>> +	nbp_vlan_delete(p, vid, flags);
>>   	return err;
>>   }
>>
>> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	if (!pve)
>>   		return -ENOENT;
>>
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		nbp_vlan_delete_untagged(p, pve->vlan);
>> +
>>   	/* Remove VLAN from the device filter if it is supported. */
>>   	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>>   	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   			pr_warn("failed to kill vid %d for device %s\n",
>>   				vid, dev->name);
>>   	}
>> +
>>   	pve->vid = BR_INVALID_VID;
>>
>>   	vlan = pve->vlan;
>> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	list_del_rcu(&pve->list);
>>   	kfree_rcu(pve, rcu);
>>
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(p->br, vlan, flags);
>>
>>   	return 0;
>>   }
>> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>>
>>   	ASSERT_RTNL();
>>
>> -	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
>> -		nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
>> +	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)  {
>> +		nbp_vlan_delete(p, pve->vid,
>> +				(BRIDGE_VLAN_INFO_MASTER |
>> +				 BRIDGE_VLAN_INFO_UNTAGGED));
>> +	}
>>   }
>>
>>   static void release_nbp(struct kobject *kobj)
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9cf2879..1b302ce 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>   			if (p)
>>   				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>   			else {
>> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> +				u16 flags = vinfo->flags |
>> +					    BRIDGE_VLAN_INFO_MASTER;
>>   				if (!br_vlan_add(br, vinfo->vid, flags))
>>   					err = -ENOMEM;
>>   			}
>> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>   				err = nbp_vlan_delete(p, vinfo->vid,
>>   						      vinfo->flags);
>>   			else {
>> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> +				u16 flags = vinfo->flags |
>> +					    BRIDGE_VLAN_INFO_MASTER;
>>   				err = br_vlan_delete(br, vinfo->vid, flags);
>>   			}
>>   			break;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index cc75212..9328463 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -179,6 +179,7 @@ struct net_bridge_port
>>   	struct netpoll			*np;
>>   #endif
>>   	struct list_head		vlan_list;
>> +	struct net_bridge_vlan __rcu	*untagged;
>>   };
>>
>>   #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>> @@ -298,6 +299,7 @@ struct net_bridge
>>   	struct timer_list		gc_timer;
>>   	struct kobject			*ifobj;
>>   	struct hlist_head		vlan_hlist[BR_VID_HASH_SIZE];
>> +	struct net_bridge_vlan __rcu	*untagged;
>>   };
>>
>>   struct br_input_skb_cb {
>> --
>> 1.7.7.6

^ permalink raw reply

* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
From: Michael S. Tsirkin @ 2012-12-18 23:10 UTC (permalink / raw)
  To: Vlad Yasevich; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
In-Reply-To: <50D0F63D.9090807@redhat.com>

On Tue, Dec 18, 2012 at 06:03:25PM -0500, Vlad Yasevich wrote:
> On 12/18/2012 06:01 PM, Michael S. Tsirkin wrote:
> >On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
> >>A user may designate a certain vlan as untagged.  This means that
> >>any ingress frame is assigned to this vlan and any forwarding decisions
> >>are made with this vlan in mind.  On egress, any frames tagged/labeled
> >>with untagged vlan have the vlan tag removed and are send as regular
> >>ethernet frames.
> >>
> >>Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
> >>---
> >>  include/uapi/linux/if_bridge.h |    3 +
> >>  net/bridge/br_if.c             |  146 +++++++++++++++++++++++++++++++++++++---
> >>  net/bridge/br_netlink.c        |    6 +-
> >>  net/bridge/br_private.h        |    2 +
> >>  4 files changed, 144 insertions(+), 13 deletions(-)
> >>
> >>diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
> >>index d0b4f5c..988d858 100644
> >>--- a/include/uapi/linux/if_bridge.h
> >>+++ b/include/uapi/linux/if_bridge.h
> >>@@ -127,6 +127,9 @@ enum {
> >>  	BR_VLAN_DEL,
> >>  };
> >>
> >>+#define BRIDGE_VLAN_INFO_MASTER		1
> >>+#define BRIDGE_VLAN_INFO_UNTAGGED	2
> >>+
> >>  struct bridge_vlan_info {
> >>  	u16 op_code;
> >>  	u16 flags;
> >>diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
> >>index 57bbb35..14563fb 100644
> >>--- a/net/bridge/br_if.c
> >>+++ b/net/bridge/br_if.c
> >>@@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
> >>  		br_vlan_destroy(vlan);
> >>  }
> >>
> >>+/* Must be protected by RTNL */
> >>+static void br_vlan_add_untagged(struct net_bridge *br,
> >>+				 struct net_bridge_vlan *vlan)
> >>+{
> >>+	ASSERT_RTNL();
> >>+	if (br->untagged == vlan)
> >>+		return;
> >>+	else if (br->untagged) {
> >>+		/* Untagged vlan is already set on the master,
> >>+		 * so drop the ref since we'll be replacing it.
> >>+		 */
> >>+		br_vlan_put(br->untagged);
> >>+	}
> >>+	br_vlan_hold(vlan);
> >>+	rcu_assign_pointer(br->untagged, vlan);
> >
> >Is there a reason for rcu here but not else where? If all users are under
> >rtnl you can just assign in a simple way.
> >If not then rcu_dereference_protected would be more appropriate.
> 
> Everywhere that the pointer changes rcu_assign_pointer is used.
> 
> Now, if we hold an RTNL, we can technically read the pointer with
> rcu since it's guaranteed not to change since it only changes under
> RTNL.
> I'll check that this is consistent.

Check what rcu_dereference_protected does. It's really just
an explicit way to say "this is accessed without rcu because I have
this lock".

> If I access the pointer without rtnl, it's always inside rcu
> critical section and with rcu_dereference().
> 
> I thought those were the basic rules of rcu.  Did that change?
> 
> -vlad



> >
> >>+}
> >>+
> >>+/* Must be protected by RTNL */
> >>+static void br_vlan_del_untagged(struct net_bridge *br,
> >>+				 struct net_bridge_vlan *vlan)
> >>+{
> >>+	ASSERT_RTNL();
> >>+	if (br->untagged == vlan) {
> >>+		br_vlan_put(vlan);
> >>+		rcu_assign_pointer(br->untagged, NULL);
> >>+	}
> >>+}
> >>+
> >>  struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
> >>  {
> >>  	struct net_bridge_vlan *vlan;
> >>@@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >>
> >>  	vlan = br_vlan_find(br, vid);
> >>  	if (vlan)
> >>-		return vlan;
> >>+		goto untagged;
> >>
> >>  	vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
> >>  	if (!vlan)
> >>@@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >>  	vlan->vid = vid;
> >>  	atomic_set(&vlan->refcnt, 1);
> >>
> >>-	if (flags & BRIDGE_FLAGS_SELF) {
> >>+	if (flags & BRIDGE_VLAN_INFO_MASTER) {
> >>  		/* Set bit 0 that is associated with the bridge master
> >>  		 * device.  Port numbers start with 1.
> >>  		 */
> >>@@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
> >>  	}
> >>
> >>  	hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
> >>+
> >>+untagged:
> >>+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+		br_vlan_add_untagged(br, vlan);
> >>+
> >>  	return vlan;
> >>  }
> >>
> >>  /* Must be protected by RTNL */
> >>-static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> >>+static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
> >>+			u16 flags)
> >>  {
> >>  	ASSERT_RTNL();
> >>
> >>-	if (flags & BRIDGE_FLAGS_SELF) {
> >>+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+		br_vlan_del_untagged(br, vlan);
> >>+
> >>+	if (flags & BRIDGE_VLAN_INFO_MASTER) {
> >>  		/* Clear bit 0 that is associated with the bridge master
> >>  		 * device.
> >>  		 */
> >>@@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
> >>
> >>  	vlan->vid = BR_INVALID_VID;
> >>
> >>+	/* If, for whatever reason, bridge still has a ref on this vlan
> >>+	 * through the @untagged pointer, drop that ref and clear untagged.
> >>+	 */
> >>+	if (br->untagged == vlan) {
> >>+		br_vlan_put(vlan);
> >>+		rcu_assign_pointer(br->untagged, NULL);
> >>+	}
> >>+
> >>  	/* Drop the self-ref to trigger descrution. */
> >>  	br_vlan_put(vlan);
> >>  }
> >>@@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
> >>  	if (!vlan)
> >>  		return -ENOENT;
> >>
> >>-	br_vlan_del(vlan, flags);
> >>+	br_vlan_del(br, vlan, flags);
> >>  	return 0;
> >>  }
> >>
> >>@@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
> >>  	for (i = 0; i < BR_VID_HASH_SIZE; i++) {
> >>  		hlist_for_each_entry_safe(vlan, node, tmp,
> >>  					  &br->vlan_hlist[i], hlist) {
> >>-			br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
> >>+			br_vlan_del(br, vlan,
> >>+				    (BRIDGE_VLAN_INFO_MASTER |
> >>+				     BRIDGE_VLAN_INFO_UNTAGGED));
> >>  		}
> >>  	}
> >>  }
> >>@@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
> >>  	return NULL;
> >>  }
> >>
> >>+static int nbp_vlan_add_untagged(struct net_bridge_port *p,
> >>+			  struct net_bridge_vlan *vlan,
> >>+			  u16 flags)
> >>+{
> >>+	struct net_device *dev = p->dev;
> >>+
> >>+	if (p->untagged) {
> >>+		/* Port already has untagged vlan set.  Drop the ref
> >>+		 * to the old one since we'll be replace it.
> >>+		 */
> >>+		br_vlan_put(p->untagged);
> >>+	} else {
> >>+		int err;
> >>+
> >>+		/* Add vid 0 to filter if filter is available. */
> >>+		if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >>+		    dev->netdev_ops->ndo_vlan_rx_add_vid &&
> >>+		    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>+			err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
> >>+			if (err)
> >>+				return err;
> >>+		}
> >>+	}
> >>+
> >>+	/* This VLAN is handled as untagged/native. Save an
> >>+	 * additional ref.
> >>+	 */
> >>+	br_vlan_hold(vlan);
> >>+	rcu_assign_pointer(p->untagged, vlan);
> >>+
> >>+	return 0;
> >>+}
> >>+
> >>+static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
> >>+				     struct net_bridge_vlan *vlan)
> >>+{
> >>+	if (p->untagged != vlan)
> >>+		return;
> >>+
> >>+	/* Remove VLAN from the device filter if it is supported. */
> >>+	if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >>+	    p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>+		int err;
> >>+
> >>+		err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
> >>+		if (err) {
> >>+			pr_warn("failed to kill vid %d for device %s\n",
> >>+				vlan->vid, p->dev->name);
> >>+		}
> >>+	}
> >>+
> >>+	/* If this VLAN is currently functioning as untagged, clear it.
> >>+	 * It's safe to drop the refcount, since the vlan is still held
> >>+	 * by the port.
> >>+	 */
> >>+	br_vlan_put(vlan);
> >>+	rcu_assign_pointer(p->untagged, NULL);
> >>+
> >>+}
> >>+
> >>  /* Must be protected by RTNL */
> >>  int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> >>  {
> >>-	struct net_port_vlan *pve;
> >>+	struct net_port_vlan *pve = NULL;
> >>  	struct net_bridge_vlan *vlan;
> >>  	struct net_device *dev = p->dev;
> >>  	int err;
> >>@@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
> >>  	set_bit(p->port_no, vlan->port_bitmap);
> >>
> >>  	list_add_tail_rcu(&pve->list, &p->vlan_list);
> >>+
> >>+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
> >>+		err = nbp_vlan_add_untagged(p, vlan, flags);
> >>+		if (err)
> >>+			goto del_vlan;
> >>+	}
> >>+
> >>  	return 0;
> >>
> >>  clean_up:
> >>  	kfree(pve);
> >>-	br_vlan_del(vlan, flags);
> >>+	br_vlan_del(p->br, vlan, flags);
> >>+	return err;
> >>+del_vlan:
> >>+	nbp_vlan_delete(p, vid, flags);
> >>  	return err;
> >>  }
> >>
> >>@@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >>  	if (!pve)
> >>  		return -ENOENT;
> >>
> >>+	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> >>+		nbp_vlan_delete_untagged(p, pve->vlan);
> >>+
> >>  	/* Remove VLAN from the device filter if it is supported. */
> >>  	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
> >>  	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
> >>@@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >>  			pr_warn("failed to kill vid %d for device %s\n",
> >>  				vid, dev->name);
> >>  	}
> >>+
> >>  	pve->vid = BR_INVALID_VID;
> >>
> >>  	vlan = pve->vlan;
> >>@@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
> >>  	list_del_rcu(&pve->list);
> >>  	kfree_rcu(pve, rcu);
> >>
> >>-	br_vlan_del(vlan, flags);
> >>+	br_vlan_del(p->br, vlan, flags);
> >>
> >>  	return 0;
> >>  }
> >>@@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
> >>
> >>  	ASSERT_RTNL();
> >>
> >>-	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
> >>-		nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
> >>+	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)  {
> >>+		nbp_vlan_delete(p, pve->vid,
> >>+				(BRIDGE_VLAN_INFO_MASTER |
> >>+				 BRIDGE_VLAN_INFO_UNTAGGED));
> >>+	}
> >>  }
> >>
> >>  static void release_nbp(struct kobject *kobj)
> >>diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
> >>index 9cf2879..1b302ce 100644
> >>--- a/net/bridge/br_netlink.c
> >>+++ b/net/bridge/br_netlink.c
> >>@@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> >>  			if (p)
> >>  				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
> >>  			else {
> >>-				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> >>+				u16 flags = vinfo->flags |
> >>+					    BRIDGE_VLAN_INFO_MASTER;
> >>  				if (!br_vlan_add(br, vinfo->vid, flags))
> >>  					err = -ENOMEM;
> >>  			}
> >>@@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
> >>  				err = nbp_vlan_delete(p, vinfo->vid,
> >>  						      vinfo->flags);
> >>  			else {
> >>-				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
> >>+				u16 flags = vinfo->flags |
> >>+					    BRIDGE_VLAN_INFO_MASTER;
> >>  				err = br_vlan_delete(br, vinfo->vid, flags);
> >>  			}
> >>  			break;
> >>diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
> >>index cc75212..9328463 100644
> >>--- a/net/bridge/br_private.h
> >>+++ b/net/bridge/br_private.h
> >>@@ -179,6 +179,7 @@ struct net_bridge_port
> >>  	struct netpoll			*np;
> >>  #endif
> >>  	struct list_head		vlan_list;
> >>+	struct net_bridge_vlan __rcu	*untagged;
> >>  };
> >>
> >>  #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
> >>@@ -298,6 +299,7 @@ struct net_bridge
> >>  	struct timer_list		gc_timer;
> >>  	struct kobject			*ifobj;
> >>  	struct hlist_head		vlan_hlist[BR_VID_HASH_SIZE];
> >>+	struct net_bridge_vlan __rcu	*untagged;
> >>  };
> >>
> >>  struct br_input_skb_cb {
> >>--
> >>1.7.7.6

^ permalink raw reply

* Re: [RFC PATCH v3 2/2] tun: fix LSM/SELinux labeling of tun/tap devices
From: Michael S. Tsirkin @ 2012-12-18 23:08 UTC (permalink / raw)
  To: Paul Moore; +Cc: netdev, linux-security-module, selinux, jasowang
In-Reply-To: <20121218225351.16104.48513.stgit@localhost>

On Tue, Dec 18, 2012 at 05:53:52PM -0500, Paul Moore wrote:
> This patch corrects some problems with LSM/SELinux that were introduced
> with the multiqueue patchset.  The problem stems from the fact that the
> multiqueue work changed the relationship between the tun device and its
> associated socket; before the socket persisted for the life of the
> device, however after the multiqueue changes the socket only persisted
> for the life of the userspace connection (fd open).  For non-persistent
> devices this is not an issue, but for persistent devices this can cause
> the tun device to lose its SELinux label.
> 
> We correct this problem by adding an opaque LSM security blob to the
> tun device struct which allows us to have the LSM security state, e.g.
> SELinux labeling information, persist for the lifetime of the tun
> device.  In the process we tweak the LSM hooks to work with this new
> approach to TUN device/socket labeling and introduce a new LSM hook,
> security_tun_dev_attach_queue(), to approve requests to attach to a
> TUN queue via TUNSETQUEUE.
> 
> The SELinux code has been adjusted to match the new LSM hooks, the
> other LSMs do not make use of the LSM TUN controls.  This patch makes
> use of the recently added "tun_socket:attach_queue" permission to
> restrict access to the TUNSETQUEUE operation.  On older SELinux
> policies which do not define the "tun_socket:attach_queue" permission
> the access control decision for TUNSETQUEUE will be handled according
> to the SELinux policy's unknown permission setting.
> 
> Signed-off-by: Paul Moore <pmoore@redhat.com>


Looks good to me. A comment not directly related to this patch, below.

> ---
>  drivers/net/tun.c                 |   27 +++++++++++++----
>  include/linux/security.h          |   59 +++++++++++++++++++++++++++++--------
>  security/capability.c             |   24 +++++++++++++--
>  security/security.c               |   28 ++++++++++++++----
>  security/selinux/hooks.c          |   50 ++++++++++++++++++++++++-------
>  security/selinux/include/objsec.h |    4 +++
>  6 files changed, 154 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 504f7f1..4b7754c 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -186,6 +186,7 @@ struct tun_struct {
>  	unsigned long ageing_time;
>  	unsigned int numdisabled;
>  	struct list_head disabled;
> +	void *security;
>  };
>  
>  static inline u32 tun_hashfn(u32 rxhash)
> @@ -496,6 +497,10 @@ static int tun_attach(struct tun_struct *tun, struct file *file)
>  	struct tun_file *tfile = file->private_data;
>  	int err;
>  
> +	err = security_tun_dev_attach(tfile->socket.sk, tun->security);
> +	if (err < 0)
> +		goto out;
> +
>  	err = -EINVAL;
>  	if (rcu_dereference_protected(tfile->tun, lockdep_rtnl_is_held()))
>  		goto out;
> @@ -1389,6 +1394,7 @@ static void tun_free_netdev(struct net_device *dev)
>  
>  	BUG_ON(!(list_empty(&tun->disabled)));
>  	tun_flow_uninit(tun);
> +	security_tun_dev_free_security(tun->security);
>  	free_netdev(dev);
>  }
>  
> @@ -1575,7 +1581,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  
>  		if (tun_not_capable(tun))
>  			return -EPERM;
> -		err = security_tun_dev_attach(tfile->socket.sk);
> +		err = security_tun_dev_open(tun->security);
>  		if (err < 0)
>  			return err;
>  
> @@ -1632,7 +1638,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
>  
>  		spin_lock_init(&tun->lock);
>  
> -		security_tun_dev_post_create(&tfile->sk);
> +		err = security_tun_dev_alloc_security(&tun->security);
> +		if (err < 0)
> +			goto err_free_dev;
>  
>  		tun_net_init(dev);
>  
> @@ -1805,12 +1813,18 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>  
>  	if (ifr->ifr_flags & IFF_ATTACH_QUEUE) {
>  		tun = tfile->detached;
> -		if (!tun)
> +		if (!tun) {
>  			ret = -EINVAL;
> -		else if (tun_not_capable(tun))
> +			goto unlock;
> +		}
> +		if (tun_not_capable(tun)) {

By the way I don't think we need to limit set_queue
by tun_not_capable. It simply makes a queue active/inactive
which seems harmless. Jason?

>  			ret = -EPERM;
> -		else
> -			ret = tun_attach(tun, file);
> +			goto unlock;
> +		}
> +		ret = security_tun_dev_attach_queue(tun->security);
> +		if (ret < 0)
> +			goto unlock;
> +		ret = tun_attach(tun, file);
>  	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
>  		tun = rcu_dereference_protected(tfile->tun,
>  						lockdep_rtnl_is_held());
> @@ -1821,6 +1835,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
>  	} else
>  		ret = -EINVAL;
>  
> +unlock:
>  	rtnl_unlock();
>  	return ret;
>  }
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 05e88bd..e09a87b 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -983,17 +983,29 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
>   *	tells the LSM to decrement the number of secmark labeling rules loaded
>   * @req_classify_flow:
>   *	Sets the flow's sid to the openreq sid.
> + * @tun_dev_alloc_security:
> + *	This hook allows a module to allocate a security structure for a TUN
> + *	device.
> + *	@security pointer to a security structure pointer.
> + *	Returns a zero on success, negative values on failure.
> + * @tun_dev_free_security:
> + *	This hook allows a module to free the security structure for a TUN
> + *	device.
> + *	@security pointer to the TUN device's security structure
>   * @tun_dev_create:
>   *	Check permissions prior to creating a new TUN device.
> - * @tun_dev_post_create:
> - *	This hook allows a module to update or allocate a per-socket security
> - *	structure.
> - *	@sk contains the newly created sock structure.
> + * @tun_dev_attach_queue:
> + *	Check permissions prior to attaching to a TUN device queue.
> + *	@security pointer to the TUN device's security structure.
>   * @tun_dev_attach:
> - *	Check permissions prior to attaching to a persistent TUN device.  This
> - *	hook can also be used by the module to update any security state
> + *	This hook can be used by the module to update any security state
>   *	associated with the TUN device's sock structure.
>   *	@sk contains the existing sock structure.
> + *	@security pointer to the TUN device's security structure.
> + * @tun_dev_open:
> + *	This hook can be used by the module to update any security state
> + *	associated with the TUN device's security structure.
> + *	@security pointer to the TUN devices's security structure.
>   *
>   * Security hooks for XFRM operations.
>   *
> @@ -1613,9 +1625,12 @@ struct security_operations {
>  	void (*secmark_refcount_inc) (void);
>  	void (*secmark_refcount_dec) (void);
>  	void (*req_classify_flow) (const struct request_sock *req, struct flowi *fl);
> -	int (*tun_dev_create)(void);
> -	void (*tun_dev_post_create)(struct sock *sk);
> -	int (*tun_dev_attach)(struct sock *sk);
> +	int (*tun_dev_alloc_security) (void **security);
> +	void (*tun_dev_free_security) (void *security);
> +	int (*tun_dev_create) (void);
> +	int (*tun_dev_attach_queue) (void *security);
> +	int (*tun_dev_attach) (struct sock *sk, void *security);
> +	int (*tun_dev_open) (void *security);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> @@ -2553,9 +2568,12 @@ void security_inet_conn_established(struct sock *sk,
>  int security_secmark_relabel_packet(u32 secid);
>  void security_secmark_refcount_inc(void);
>  void security_secmark_refcount_dec(void);
> +int security_tun_dev_alloc_security(void **security);
> +void security_tun_dev_free_security(void *security);
>  int security_tun_dev_create(void);
> -void security_tun_dev_post_create(struct sock *sk);
> -int security_tun_dev_attach(struct sock *sk);
> +int security_tun_dev_attach_queue(void *security);
> +int security_tun_dev_attach(struct sock *sk, void *security);
> +int security_tun_dev_open(void *security);
>  
>  #else	/* CONFIG_SECURITY_NETWORK */
>  static inline int security_unix_stream_connect(struct sock *sock,
> @@ -2720,16 +2738,31 @@ static inline void security_secmark_refcount_dec(void)
>  {
>  }
>  
> +static inline int security_tun_dev_alloc_security(void **security)
> +{
> +	return 0;
> +}
> +
> +static inline void security_tun_dev_free_security(void *security)
> +{
> +}
> +
>  static inline int security_tun_dev_create(void)
>  {
>  	return 0;
>  }
>  
> -static inline void security_tun_dev_post_create(struct sock *sk)
> +static inline int security_tun_dev_attach_queue(void *security)
> +{
> +	return 0;
> +}
> +
> +static inline int security_tun_dev_attach(struct sock *sk, void *security)
>  {
> +	return 0;
>  }
>  
> -static inline int security_tun_dev_attach(struct sock *sk)
> +static inline int security_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> diff --git a/security/capability.c b/security/capability.c
> index b14a30c..76c1dc9 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -704,16 +704,31 @@ static void cap_req_classify_flow(const struct request_sock *req,
>  {
>  }
>  
> +static int cap_tun_dev_alloc_security(void **security)
> +{
> +	return 0;
> +}
> +
> +static void cap_tun_dev_free_security(void *security)
> +{
> +}
> +
>  static int cap_tun_dev_create(void)
>  {
>  	return 0;
>  }
>  
> -static void cap_tun_dev_post_create(struct sock *sk)
> +static int cap_tun_dev_attach_queue(void *security)
> +{
> +	return 0;
> +}
> +
> +static int cap_tun_dev_attach(struct sock *sk, void *security)
>  {
> +	return 0;
>  }
>  
> -static int cap_tun_dev_attach(struct sock *sk)
> +static int cap_tun_dev_open(void *security)
>  {
>  	return 0;
>  }
> @@ -1044,8 +1059,11 @@ void __init security_fixup_ops(struct security_operations *ops)
>  	set_to_cap_if_null(ops, secmark_refcount_inc);
>  	set_to_cap_if_null(ops, secmark_refcount_dec);
>  	set_to_cap_if_null(ops, req_classify_flow);
> +	set_to_cap_if_null(ops, tun_dev_alloc_security);
> +	set_to_cap_if_null(ops, tun_dev_free_security);
>  	set_to_cap_if_null(ops, tun_dev_create);
> -	set_to_cap_if_null(ops, tun_dev_post_create);
> +	set_to_cap_if_null(ops, tun_dev_open);
> +	set_to_cap_if_null(ops, tun_dev_attach_queue);
>  	set_to_cap_if_null(ops, tun_dev_attach);
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/security/security.c b/security/security.c
> index 8dcd4ae..a271ed4 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1244,24 +1244,42 @@ void security_secmark_refcount_dec(void)
>  }
>  EXPORT_SYMBOL(security_secmark_refcount_dec);
>  
> +int security_tun_dev_alloc_security(void **security)
> +{
> +	return security_ops->tun_dev_alloc_security(security);
> +}
> +EXPORT_SYMBOL(security_tun_dev_alloc_security);
> +
> +void security_tun_dev_free_security(void *security)
> +{
> +	security_ops->tun_dev_free_security(security);
> +}
> +EXPORT_SYMBOL(security_tun_dev_free_security);
> +
>  int security_tun_dev_create(void)
>  {
>  	return security_ops->tun_dev_create();
>  }
>  EXPORT_SYMBOL(security_tun_dev_create);
>  
> -void security_tun_dev_post_create(struct sock *sk)
> +int security_tun_dev_attach_queue(void *security)
>  {
> -	return security_ops->tun_dev_post_create(sk);
> +	return security_ops->tun_dev_attach_queue(security);
>  }
> -EXPORT_SYMBOL(security_tun_dev_post_create);
> +EXPORT_SYMBOL(security_tun_dev_attach_queue);
>  
> -int security_tun_dev_attach(struct sock *sk)
> +int security_tun_dev_attach(struct sock *sk, void *security)
>  {
> -	return security_ops->tun_dev_attach(sk);
> +	return security_ops->tun_dev_attach(sk, security);
>  }
>  EXPORT_SYMBOL(security_tun_dev_attach);
>  
> +int security_tun_dev_open(void *security)
> +{
> +	return security_ops->tun_dev_open(security);
> +}
> +EXPORT_SYMBOL(security_tun_dev_open);
> +
>  #endif	/* CONFIG_SECURITY_NETWORK */
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 61a5336..ef26e96 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4399,6 +4399,24 @@ static void selinux_req_classify_flow(const struct request_sock *req,
>  	fl->flowi_secid = req->secid;
>  }
>  
> +static int selinux_tun_dev_alloc_security(void **security)
> +{
> +	struct tun_security_struct *tunsec;
> +
> +	tunsec = kzalloc(sizeof(*tunsec), GFP_KERNEL);
> +	if (!tunsec)
> +		return -ENOMEM;
> +	tunsec->sid = current_sid();
> +
> +	*security = tunsec;
> +	return 0;
> +}
> +
> +static void selinux_tun_dev_free_security(void *security)
> +{
> +	kfree(security);
> +}
> +
>  static int selinux_tun_dev_create(void)
>  {
>  	u32 sid = current_sid();
> @@ -4414,8 +4432,17 @@ static int selinux_tun_dev_create(void)
>  			    NULL);
>  }
>  
> -static void selinux_tun_dev_post_create(struct sock *sk)
> +static int selinux_tun_dev_attach_queue(void *security)
>  {
> +	struct tun_security_struct *tunsec = security;
> +
> +	return avc_has_perm(current_sid(), tunsec->sid, SECCLASS_TUN_SOCKET,
> +			    TUN_SOCKET__ATTACH_QUEUE, NULL);
> +}
> +
> +static int selinux_tun_dev_attach(struct sock *sk, void *security)
> +{
> +	struct tun_security_struct *tunsec = security;
>  	struct sk_security_struct *sksec = sk->sk_security;
>  
>  	/* we don't currently perform any NetLabel based labeling here and it
> @@ -4425,20 +4452,19 @@ static void selinux_tun_dev_post_create(struct sock *sk)
>  	 * cause confusion to the TUN user that had no idea network labeling
>  	 * protocols were being used */
>  
> -	/* see the comments in selinux_tun_dev_create() about why we don't use
> -	 * the sockcreate SID here */
> -
> -	sksec->sid = current_sid();
> +	sksec->sid = tunsec->sid;
>  	sksec->sclass = SECCLASS_TUN_SOCKET;
> +
> +	return 0;
>  }
>  
> -static int selinux_tun_dev_attach(struct sock *sk)
> +static int selinux_tun_dev_open(void *security)
>  {
> -	struct sk_security_struct *sksec = sk->sk_security;
> +	struct tun_security_struct *tunsec = security;
>  	u32 sid = current_sid();
>  	int err;
>  
> -	err = avc_has_perm(sid, sksec->sid, SECCLASS_TUN_SOCKET,
> +	err = avc_has_perm(sid, tunsec->sid, SECCLASS_TUN_SOCKET,
>  			   TUN_SOCKET__RELABELFROM, NULL);
>  	if (err)
>  		return err;
> @@ -4446,8 +4472,7 @@ static int selinux_tun_dev_attach(struct sock *sk)
>  			   TUN_SOCKET__RELABELTO, NULL);
>  	if (err)
>  		return err;
> -
> -	sksec->sid = sid;
> +	tunsec->sid = sid;
>  
>  	return 0;
>  }
> @@ -5642,9 +5667,12 @@ static struct security_operations selinux_ops = {
>  	.secmark_refcount_inc =		selinux_secmark_refcount_inc,
>  	.secmark_refcount_dec =		selinux_secmark_refcount_dec,
>  	.req_classify_flow =		selinux_req_classify_flow,
> +	.tun_dev_alloc_security =	selinux_tun_dev_alloc_security,
> +	.tun_dev_free_security =	selinux_tun_dev_free_security,
>  	.tun_dev_create =		selinux_tun_dev_create,
> -	.tun_dev_post_create = 		selinux_tun_dev_post_create,
> +	.tun_dev_attach_queue =		selinux_tun_dev_attach_queue,
>  	.tun_dev_attach =		selinux_tun_dev_attach,
> +	.tun_dev_open =			selinux_tun_dev_open,
>  
>  #ifdef CONFIG_SECURITY_NETWORK_XFRM
>  	.xfrm_policy_alloc_security =	selinux_xfrm_policy_alloc,
> diff --git a/security/selinux/include/objsec.h b/security/selinux/include/objsec.h
> index 26c7eee..aa47bca 100644
> --- a/security/selinux/include/objsec.h
> +++ b/security/selinux/include/objsec.h
> @@ -110,6 +110,10 @@ struct sk_security_struct {
>  	u16 sclass;			/* sock security class */
>  };
>  
> +struct tun_security_struct {
> +	u32 sid;			/* SID for the tun device sockets */
> +};
> +
>  struct key_security_struct {
>  	u32 sid;	/* SID of key */
>  };

^ permalink raw reply

* Re: [PATCH net-next] xfrm: removes a superfluous check and add a statistic
From: David Miller @ 2012-12-18 23:38 UTC (permalink / raw)
  To: roy.qing.li; +Cc: netdev
In-Reply-To: <1355819949-30429-1-git-send-email-roy.qing.li@gmail.com>

From: roy.qing.li@gmail.com
Date: Tue, 18 Dec 2012 16:39:09 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> Remove the check if x->km.state equal to XFRM_STATE_VALID in
> xfrm_state_check_expire(), which will be done before call
> xfrm_state_check_expire().
> 
> add a LINUX_MIB_XFRMOUTSTATEINVALID statistic to record the
> outbound error due to invalid xfrm state.
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Please add new statistic SNMP mib numbers to the end of the
enumeration, rather than in the middle.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2] be2net: fix be_close() to ensure all events are ack'ed
From: David Miller @ 2012-12-19  0:19 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <f1923deb-9df8-4c1e-97be-944fd119e2ce@CMEXHTCAS2.ad.emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Tue, 18 Dec 2012 11:08:50 +0530

> In be_close(), be_eq_clean() must be called after all RX/TX/MCC queues
> have been cleaned to ensure that any events caused while cleaning up
> completions are notified/acked. Not clearing all events can cause
> upredictable behaviour when RX rings are re-created in the subsequent
> be_open().
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] be2net: fix wrong frag_idx reported by RX CQ
From: David Miller @ 2012-12-19  0:19 UTC (permalink / raw)
  To: sathya.perla; +Cc: netdev
In-Reply-To: <42b4ddf1-1024-4762-b86c-66c033afc219@CMEXHTCAS2.ad.emulex.com>

From: Sathya Perla <sathya.perla@emulex.com>
Date: Tue, 18 Dec 2012 11:08:51 +0530

> The RX CQ can report completions with invalid frag_idx when the RXQ that
> was *previously* using it, was not cleaned up properly. This hits
> a BUG_ON() in be2net.
> 
> When completion coalescing is enabled on a CQ, an explicit CQ-notify
> (with rearm) is needed for each compl, to flush partially coalesced CQ
> entries that are pending DMA.
> 
> In be_close(), this fix now notifies CQ for each compl, waits explicitly
> for the flush compl to arrive and complains if it doesn't arrive.
> 
> Also renaming be_crit_error() to be_hw_error() as it's the more
> appropriate name and to convey that we don't wait for the flush compl
> only when a HW error has occurred.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Applied.

^ permalink raw reply

* Re: [GIT PULL net-next 01/17] ndisc: Fix size calculation for headers.
From: David Miller @ 2012-12-19  0:23 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev
In-Reply-To: <50D04AD4.8010908@linux-ipv6.org>

From: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Date: Tue, 18 Dec 2012 19:52:04 +0900

> We used to allocate MAX_HEADER bytes more than needed but
> reserved hlen only (not MAX_HEADER + hlen) and the MAX_HEADER
> was left behind.
> 
> Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>

This change is wrong.

The MAX_HEADER is being used in order to accomodate any
possible encapsulation that may occur.

I'm really disappointed that the very first patch in this series is
buggy, and you didn't even bother to repost the absolutely required
"00/17" email for this series which is where you're supposed to give a
top-level overview of your changes as well as any GIT pull request.

^ permalink raw reply

* Re: [GIT PULL net-next 01/17] ndisc: Fix size calculation for headers.
From: David Miller @ 2012-12-19  0:24 UTC (permalink / raw)
  To: yoshfuji; +Cc: netdev
In-Reply-To: <20121218.162338.1594192050394527214.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Tue, 18 Dec 2012 16:23:38 -0800 (PST)

> I'm really disappointed that the very first patch in this series is
> buggy, and you didn't even bother to repost the absolutely required
> "00/17" email for this series which is where you're supposed to give a
> top-level overview of your changes as well as any GIT pull request.

Also, right now the net-next tree is not open, so these changes
aren't even appropriate to be submitting right now.

^ permalink raw reply

* Re: [PATCH 0/2] smsc95xx enhancements
From: David Miller @ 2012-12-19  0:25 UTC (permalink / raw)
  To: steve.glendinning; +Cc: netdev, ming.lei, oneukum, gregkh
In-Reply-To: <1355827574-15164-1-git-send-email-steve.glendinning@shawell.net>

From: Steve Glendinning <steve.glendinning@shawell.net>
Date: Tue, 18 Dec 2012 10:46:12 +0000

> Two driver enhancements for net-next.  There's ongoing discussion around
> the best way to improve autosuspend moving forwards but in the meantime
> the second patch in this set enables autosuspend for supported parts in
> most situations.

Please do not submit net-next changes until the net-next tree is
actually open and available for submissions.

^ permalink raw reply

* Re: [PATCH] net: fec: forbid FEC_PTP on SoCs that do not support
From: David Miller @ 2012-12-19  0:26 UTC (permalink / raw)
  To: shawn.guo; +Cc: netdev, s.hauer, richardcochran
In-Reply-To: <1355836004-22067-1-git-send-email-shawn.guo@linaro.org>

From: Shawn Guo <shawn.guo@linaro.org>
Date: Tue, 18 Dec 2012 21:06:44 +0800

> Beside imx6q, the kernel built from imx_v6_v7_defconfig is also
> supposed to be running on other IMX SoCs that do not have the PTP
> block.  Before fec driver gets fixed to run-time detect target hardware
> rather than conditional compiling with #ifdef CONFIG_FEC_PTP, let's
> give it a quick fix in Kconfig to forbid FEC_PTP on those IMX SoCs that
> do not support PTP.
> 
> Reported-by: Sascha Hauer <s.hauer@pengutronix.de>
> Signed-off-by: Shawn Guo <shawn.guo@linaro.org>

This uglyness is exactly why this needs to be detected at run
time.

Anyways, applied.

^ permalink raw reply

* Re: [PATCH V2 09/12] bridge: Add the ability to configure untagged vlans
From: Vlad Yasevich @ 2012-12-19  1:06 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, shemminger, davem, or.gerlitz, jhs
In-Reply-To: <20121218230407.GB1135@redhat.com>

On 12/18/2012 06:04 PM, Michael S. Tsirkin wrote:
> On Tue, Dec 18, 2012 at 02:01:00PM -0500, Vlad Yasevich wrote:
>> A user may designate a certain vlan as untagged.  This means that
>> any ingress frame is assigned to this vlan and any forwarding decisions
>> are made with this vlan in mind.  On egress, any frames tagged/labeled
>> with untagged vlan have the vlan tag removed and are send as regular
>> ethernet frames.
>>
>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>> ---
>>   include/uapi/linux/if_bridge.h |    3 +
>>   net/bridge/br_if.c             |  146 +++++++++++++++++++++++++++++++++++++---
>>   net/bridge/br_netlink.c        |    6 +-
>>   net/bridge/br_private.h        |    2 +
>>   4 files changed, 144 insertions(+), 13 deletions(-)
>>
>> diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
>> index d0b4f5c..988d858 100644
>> --- a/include/uapi/linux/if_bridge.h
>> +++ b/include/uapi/linux/if_bridge.h
>> @@ -127,6 +127,9 @@ enum {
>>   	BR_VLAN_DEL,
>>   };
>>
>> +#define BRIDGE_VLAN_INFO_MASTER		1
>> +#define BRIDGE_VLAN_INFO_UNTAGGED	2
>> +
>>   struct bridge_vlan_info {
>>   	u16 op_code;
>>   	u16 flags;
>> diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
>> index 57bbb35..14563fb 100644
>> --- a/net/bridge/br_if.c
>> +++ b/net/bridge/br_if.c
>> @@ -108,6 +108,34 @@ static void br_vlan_put(struct net_bridge_vlan *vlan)
>>   		br_vlan_destroy(vlan);
>>   }
>>
>> +/* Must be protected by RTNL */
>> +static void br_vlan_add_untagged(struct net_bridge *br,
>> +				 struct net_bridge_vlan *vlan)
>> +{
>> +	ASSERT_RTNL();
>> +	if (br->untagged == vlan)
>> +		return;
>> +	else if (br->untagged) {
>> +		/* Untagged vlan is already set on the master,
>> +		 * so drop the ref since we'll be replacing it.
>> +		 */
>> +		br_vlan_put(br->untagged);
>> +	}
>> +	br_vlan_hold(vlan);
>> +	rcu_assign_pointer(br->untagged, vlan);
>> +}
>> +
>> +/* Must be protected by RTNL */
>> +static void br_vlan_del_untagged(struct net_bridge *br,
>> +				 struct net_bridge_vlan *vlan)
>> +{
>> +	ASSERT_RTNL();
>> +	if (br->untagged == vlan) {
>> +		br_vlan_put(vlan);
>> +		rcu_assign_pointer(br->untagged, NULL);
>> +	}
>> +}
>> +
>>   struct net_bridge_vlan *br_vlan_find(struct net_bridge *br, u16 vid)
>>   {
>>   	struct net_bridge_vlan *vlan;
>> @@ -132,7 +160,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>
>>   	vlan = br_vlan_find(br, vid);
>>   	if (vlan)
>> -		return vlan;
>> +		goto untagged;
>>
>>   	vlan = kzalloc(sizeof(struct net_bridge_vlan), GFP_KERNEL);
>>   	if (!vlan)
>> @@ -141,7 +169,7 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>   	vlan->vid = vid;
>>   	atomic_set(&vlan->refcnt, 1);
>>
>> -	if (flags & BRIDGE_FLAGS_SELF) {
>> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>   		/* Set bit 0 that is associated with the bridge master
>>   		 * device.  Port numbers start with 1.
>>   		 */
>> @@ -149,15 +177,24 @@ struct net_bridge_vlan *br_vlan_add(struct net_bridge *br, u16 vid,
>>   	}
>>
>>   	hlist_add_head_rcu(&vlan->hlist, &br->vlan_hlist[br_vlan_hash(vid)]);
>> +
>> +untagged:
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		br_vlan_add_untagged(br, vlan);
>> +
>>   	return vlan;
>>   }
>>
>>   /* Must be protected by RTNL */
>> -static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>> +static void br_vlan_del(struct net_bridge *br, struct net_bridge_vlan *vlan,
>> +			u16 flags)
>>   {
>>   	ASSERT_RTNL();
>>
>> -	if (flags & BRIDGE_FLAGS_SELF) {
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		br_vlan_del_untagged(br, vlan);
>> +
>> +	if (flags & BRIDGE_VLAN_INFO_MASTER) {
>>   		/* Clear bit 0 that is associated with the bridge master
>>   		 * device.
>>   		 */
>> @@ -172,6 +209,14 @@ static void br_vlan_del(struct net_bridge_vlan *vlan, u16 flags)
>>
>>   	vlan->vid = BR_INVALID_VID;
>>
>> +	/* If, for whatever reason, bridge still has a ref on this vlan
>> +	 * through the @untagged pointer, drop that ref and clear untagged.
>> +	 */
>> +	if (br->untagged == vlan) {
>> +		br_vlan_put(vlan);
>> +		rcu_assign_pointer(br->untagged, NULL);
>
> Is something doing an rcu sync after this point?
> If yes maybe add a comment saying where it is, if not - some rcu read
> side could still be using it through this pointer.

yes, at this point, all references from the ports have been dropped and
we are trying to destroy the element.  The put below will trigger the
destruction path which will do kfree_rcu().  Thus there is a grace
period...

-vlad

>
>> +	}
>> +
>>   	/* Drop the self-ref to trigger descrution. */
>>   	br_vlan_put(vlan);
>>   }
>> @@ -187,7 +232,7 @@ int br_vlan_delete(struct net_bridge *br, u16 vid, u16 flags)
>>   	if (!vlan)
>>   		return -ENOENT;
>>
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(br, vlan, flags);
>>   	return 0;
>>   }
>>
>> @@ -204,7 +249,9 @@ static void br_vlan_flush(struct net_bridge *br)
>>   	for (i = 0; i < BR_VID_HASH_SIZE; i++) {
>>   		hlist_for_each_entry_safe(vlan, node, tmp,
>>   					  &br->vlan_hlist[i], hlist) {
>> -			br_vlan_del(vlan, BRIDGE_FLAGS_SELF);
>> +			br_vlan_del(br, vlan,
>> +				    (BRIDGE_VLAN_INFO_MASTER |
>> +				     BRIDGE_VLAN_INFO_UNTAGGED));
>>   		}
>>   	}
>>   }
>> @@ -224,10 +271,70 @@ struct net_port_vlan *nbp_vlan_find(const struct net_bridge_port *p, u16 vid)
>>   	return NULL;
>>   }
>>
>> +static int nbp_vlan_add_untagged(struct net_bridge_port *p,
>> +			  struct net_bridge_vlan *vlan,
>> +			  u16 flags)
>> +{
>> +	struct net_device *dev = p->dev;
>> +
>> +	if (p->untagged) {
>> +		/* Port already has untagged vlan set.  Drop the ref
>> +		 * to the old one since we'll be replace it.
>> +		 */
>> +		br_vlan_put(p->untagged);
>> +	} else {
>> +		int err;
>> +
>> +		/* Add vid 0 to filter if filter is available. */
>> +		if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +		    dev->netdev_ops->ndo_vlan_rx_add_vid &&
>> +		    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> +			err = dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
>> +			if (err)
>> +				return err;
>> +		}
>> +	}
>> +
>> +	/* This VLAN is handled as untagged/native. Save an
>> +	 * additional ref.
>> +	 */
>> +	br_vlan_hold(vlan);
>> +	rcu_assign_pointer(p->untagged, vlan);
>> +
>> +	return 0;
>> +}
>> +
>> +static void nbp_vlan_delete_untagged(struct net_bridge_port *p,
>> +				     struct net_bridge_vlan *vlan)
>> +{
>> +	if (p->untagged != vlan)
>> +		return;
>> +
>> +	/* Remove VLAN from the device filter if it is supported. */
>> +	if ((p->dev->features & NETIF_F_HW_VLAN_FILTER) &&
>> +	    p->dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> +		int err;
>> +
>> +		err = p->dev->netdev_ops->ndo_vlan_rx_kill_vid(p->dev, 0);
>> +		if (err) {
>> +			pr_warn("failed to kill vid %d for device %s\n",
>> +				vlan->vid, p->dev->name);
>> +		}
>> +	}
>> +
>> +	/* If this VLAN is currently functioning as untagged, clear it.
>> +	 * It's safe to drop the refcount, since the vlan is still held
>> +	 * by the port.
>> +	 */
>> +	br_vlan_put(vlan);
>> +	rcu_assign_pointer(p->untagged, NULL);
>> +
>> +}
>> +
>>   /* Must be protected by RTNL */
>>   int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>   {
>> -	struct net_port_vlan *pve;
>> +	struct net_port_vlan *pve = NULL;
>>   	struct net_bridge_vlan *vlan;
>>   	struct net_device *dev = p->dev;
>>   	int err;
>> @@ -275,11 +382,21 @@ int nbp_vlan_add(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	set_bit(p->port_no, vlan->port_bitmap);
>>
>>   	list_add_tail_rcu(&pve->list, &p->vlan_list);
>> +
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED) {
>> +		err = nbp_vlan_add_untagged(p, vlan, flags);
>> +		if (err)
>> +			goto del_vlan;
>> +	}
>> +
>>   	return 0;
>>
>>   clean_up:
>>   	kfree(pve);
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(p->br, vlan, flags);
>> +	return err;
>> +del_vlan:
>> +	nbp_vlan_delete(p, vid, flags);
>>   	return err;
>>   }
>>
>> @@ -296,6 +413,9 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	if (!pve)
>>   		return -ENOENT;
>>
>> +	if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> +		nbp_vlan_delete_untagged(p, pve->vlan);
>> +
>>   	/* Remove VLAN from the device filter if it is supported. */
>>   	if ((dev->features & NETIF_F_HW_VLAN_FILTER) &&
>>   	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
>> @@ -306,6 +426,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   			pr_warn("failed to kill vid %d for device %s\n",
>>   				vid, dev->name);
>>   	}
>> +
>>   	pve->vid = BR_INVALID_VID;
>>
>>   	vlan = pve->vlan;
>> @@ -316,7 +437,7 @@ int nbp_vlan_delete(struct net_bridge_port *p, u16 vid, u16 flags)
>>   	list_del_rcu(&pve->list);
>>   	kfree_rcu(pve, rcu);
>>
>> -	br_vlan_del(vlan, flags);
>> +	br_vlan_del(p->br, vlan, flags);
>>
>>   	return 0;
>>   }
>> @@ -328,8 +449,11 @@ static void nbp_vlan_flush(struct net_bridge_port *p)
>>
>>   	ASSERT_RTNL();
>>
>> -	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)
>> -		nbp_vlan_delete(p, pve->vid, BRIDGE_FLAGS_SELF);
>> +	list_for_each_entry_safe(pve, tmp, &p->vlan_list, list)  {
>> +		nbp_vlan_delete(p, pve->vid,
>> +				(BRIDGE_VLAN_INFO_MASTER |
>> +				 BRIDGE_VLAN_INFO_UNTAGGED));
>> +	}
>>   }
>>
>>   static void release_nbp(struct kobject *kobj)
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9cf2879..1b302ce 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -199,7 +199,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>   			if (p)
>>   				err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>   			else {
>> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> +				u16 flags = vinfo->flags |
>> +					    BRIDGE_VLAN_INFO_MASTER;
>>   				if (!br_vlan_add(br, vinfo->vid, flags))
>>   					err = -ENOMEM;
>>   			}
>> @@ -210,7 +211,8 @@ static int br_afspec(struct net_bridge *br, struct net_bridge_port *p,
>>   				err = nbp_vlan_delete(p, vinfo->vid,
>>   						      vinfo->flags);
>>   			else {
>> -				u16 flags = vinfo->flags | BRIDGE_FLAGS_SELF;
>> +				u16 flags = vinfo->flags |
>> +					    BRIDGE_VLAN_INFO_MASTER;
>>   				err = br_vlan_delete(br, vinfo->vid, flags);
>>   			}
>>   			break;
>> diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
>> index cc75212..9328463 100644
>> --- a/net/bridge/br_private.h
>> +++ b/net/bridge/br_private.h
>> @@ -179,6 +179,7 @@ struct net_bridge_port
>>   	struct netpoll			*np;
>>   #endif
>>   	struct list_head		vlan_list;
>> +	struct net_bridge_vlan __rcu	*untagged;
>>   };
>>
>>   #define br_port_exists(dev) (dev->priv_flags & IFF_BRIDGE_PORT)
>> @@ -298,6 +299,7 @@ struct net_bridge
>>   	struct timer_list		gc_timer;
>>   	struct kobject			*ifobj;
>>   	struct hlist_head		vlan_hlist[BR_VID_HASH_SIZE];
>> +	struct net_bridge_vlan __rcu	*untagged;
>>   };
>>
>>   struct br_input_skb_cb {
>> --
>> 1.7.7.6
> --
> 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

* Good day!!!
From: Mr Kale mills @ 2012-12-19  1:14 UTC (permalink / raw)




My client Mrs.Alicia Robinson,93 years old and a philanthropist has  
willed to you $5.5 million,Email:kale_mills@live.com(strictly with  
this email address)

^ permalink raw reply

* Re: [PATCHv3 2/2] smsc95xx: enable dynamic autosuspend
From: Ming Lei @ 2012-12-19  2:23 UTC (permalink / raw)
  To: Steve Glendinning; +Cc: netdev, oneukum, gregkh
In-Reply-To: <1355827574-15164-3-git-send-email-steve.glendinning@shawell.net>

On Tue, Dec 18, 2012 at 6:46 PM, Steve Glendinning
<steve.glendinning@shawell.net> wrote:
> This patch enables USB dynamic autosuspend for LAN9500A.  This
> saves very little power in itself, but it allows power saving

Putting device into suspend1 after link being off does save power, so
please update the commit log.

> in upstream hubs/hosts.
>
> The earlier devices in this family (LAN9500/9512/9514) do not
> support this feature.
>
> Signed-off-by: Steve Glendinning <steve.glendinning@shawell.net>
> ---
>  drivers/net/usb/smsc95xx.c |  151 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 150 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c
> index 124e67f..6a74a68 100644
> --- a/drivers/net/usb/smsc95xx.c
> +++ b/drivers/net/usb/smsc95xx.c
> @@ -55,6 +55,13 @@
>  #define FEATURE_PHY_NLP_CROSSOVER      (0x02)
>  #define FEATURE_AUTOSUSPEND            (0x04)
>
> +#define SUSPEND_SUSPEND0               (0x01)
> +#define SUSPEND_SUSPEND1               (0x02)
> +#define SUSPEND_SUSPEND2               (0x04)
> +#define SUSPEND_SUSPEND3               (0x08)
> +#define SUSPEND_ALLMODES               (SUSPEND_SUSPEND0 | SUSPEND_SUSPEND1 | \
> +                                        SUSPEND_SUSPEND2 | SUSPEND_SUSPEND3)
> +
>  struct smsc95xx_priv {
>         u32 mac_cr;
>         u32 hash_hi;
> @@ -62,6 +69,7 @@ struct smsc95xx_priv {
>         u32 wolopts;
>         spinlock_t mac_cr_lock;
>         u8 features;
> +       u8 suspend_flags;
>  };
>
>  static bool turbo_mode = true;
> @@ -1242,6 +1250,8 @@ static int smsc95xx_enter_suspend0(struct usbnet *dev)
>         /* read back PM_CTRL */
>         ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND0;
> +
>         return ret;
>  }
>
> @@ -1286,11 +1296,14 @@ static int smsc95xx_enter_suspend1(struct usbnet *dev)
>
>         ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND1;
> +
>         return ret;
>  }
>
>  static int smsc95xx_enter_suspend2(struct usbnet *dev)
>  {
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
>         u32 val;
>         int ret;
>
> @@ -1303,9 +1316,97 @@ static int smsc95xx_enter_suspend2(struct usbnet *dev)
>
>         ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
>
> +       pdata->suspend_flags |= SUSPEND_SUSPEND2;
> +
>         return ret;
>  }
>
> +static int smsc95xx_enter_suspend3(struct usbnet *dev)
> +{
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +       u32 val;
> +       int ret;
> +
> +       ret = smsc95xx_read_reg_nopm(dev, RX_FIFO_INF, &val);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (val & 0xFFFF) {
> +               netdev_info(dev->net, "rx fifo not empty in autosuspend\n");
> +               return -EBUSY;
> +       }
> +
> +       ret = smsc95xx_read_reg_nopm(dev, PM_CTRL, &val);
> +       if (ret < 0)
> +               return ret;
> +
> +       val &= ~(PM_CTL_SUS_MODE_ | PM_CTL_WUPS_ | PM_CTL_PHY_RST_);
> +       val |= PM_CTL_SUS_MODE_3 | PM_CTL_RES_CLR_WKP_STS;
> +
> +       ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
> +       if (ret < 0)
> +               return ret;
> +
> +       /* clear wol status */
> +       val &= ~PM_CTL_WUPS_;
> +       val |= PM_CTL_WUPS_WOL_;
> +
> +       ret = smsc95xx_write_reg_nopm(dev, PM_CTRL, val);
> +       if (ret < 0)
> +               return ret;
> +
> +       pdata->suspend_flags |= SUSPEND_SUSPEND3;
> +
> +       return 0;
> +}
> +
> +static int smsc95xx_autosuspend(struct usbnet *dev, u32 link_up)
> +{
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +       int ret;
> +
> +       if (!netif_running(dev->net)) {
> +               /* interface is ifconfig down so fully power down hw */
> +               netdev_dbg(dev->net, "autosuspend entering SUSPEND2\n");
> +               return smsc95xx_enter_suspend2(dev);
> +       }
> +
> +       if (!link_up) {
> +               /* link is down so enter EDPD mode, but only if device can
> +                * reliably resume from it.  This check should be redundant
> +                * as current FEATURE_AUTOSUSPEND parts also support
> +                * FEATURE_PHY_NLP_CROSSOVER but it's included for clarity */
> +               if (!(pdata->features & FEATURE_PHY_NLP_CROSSOVER)) {
> +                       netdev_warn(dev->net, "EDPD not supported\n");
> +                       return -EBUSY;
> +               }
> +
> +               netdev_dbg(dev->net, "autosuspend entering SUSPEND1\n");
> +
> +               /* enable PHY wakeup events for if cable is attached */
> +               ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
> +                       PHY_INT_MASK_ANEG_COMP_);
> +               if (ret < 0) {
> +                       netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
> +                       return ret;
> +               }
> +
> +               netdev_info(dev->net, "entering SUSPEND1 mode\n");
> +               return smsc95xx_enter_suspend1(dev);
> +       }
> +
> +       /* enable PHY wakeup events so we remote wakeup if cable is pulled */
> +       ret = smsc95xx_enable_phy_wakeup_interrupts(dev,
> +               PHY_INT_MASK_LINK_DOWN_);
> +       if (ret < 0) {
> +               netdev_warn(dev->net, "error enabling PHY wakeup ints\n");
> +               return ret;
> +       }
> +
> +       netdev_dbg(dev->net, "autosuspend entering SUSPEND3\n");
> +       return smsc95xx_enter_suspend3(dev);
> +}
> +
>  static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>  {
>         struct usbnet *dev = usb_get_intfdata(intf);
> @@ -1313,15 +1414,35 @@ static int smsc95xx_suspend(struct usb_interface *intf, pm_message_t message)
>         u32 val, link_up;
>         int ret;
>
> +       /* TODO: don't indicate this feature to usb framework if
> +        * our current hardware doesn't have the capability
> +        */
> +       if ((message.event == PM_EVENT_AUTO_SUSPEND) &&
> +           (!(pdata->features & FEATURE_AUTOSUSPEND))) {
> +               netdev_warn(dev->net, "autosuspend not supported\n");
> +               return -EBUSY;
> +       }

The above is wrong, sorry for not mentioning it in the previous review.

Firstly, the flag FEATURE_AUTOSUSPEND is misleading, and it only
means that the device can't generate remote wakeup, and it shouldn't
mean that the device can't be put into auto-suspend. So a better name
might be FEATURE_REMOTE_WAKEUP.

Secondly, the device should and can be put into auto suspend when
the network interface is down, but the above change makes that
impossible.

> +
>         ret = usbnet_suspend(intf, message);
>         if (ret < 0) {
>                 netdev_warn(dev->net, "usbnet_suspend error\n");
>                 return ret;
>         }
>
> +       if (pdata->suspend_flags) {
> +               netdev_warn(dev->net, "error during last resume\n");
> +               pdata->suspend_flags = 0;
> +       }
> +
>         /* determine if link is up using only _nopm functions */
>         link_up = smsc95xx_link_ok_nopm(dev);
>
> +       if (message.event == PM_EVENT_AUTO_SUSPEND) {
> +               ret = smsc95xx_autosuspend(dev, link_up);
> +               goto done;
> +       }
> +
> +       /* if we get this far we're not autosuspending */
>         /* if no wol options set, or if link is down and we're not waking on
>          * PHY activity, enter lowest power SUSPEND2 mode
>          */
> @@ -1552,12 +1673,18 @@ static int smsc95xx_resume(struct usb_interface *intf)
>  {
>         struct usbnet *dev = usb_get_intfdata(intf);
>         struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +       u8 suspend_flags = pdata->suspend_flags;
>         int ret;
>         u32 val;
>
>         BUG_ON(!dev);
>
> -       if (pdata->wolopts) {
> +       netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
> +
> +       /* do this first to ensure it's cleared even in error case */
> +       pdata->suspend_flags = 0;
> +
> +       if (suspend_flags & SUSPEND_ALLMODES) {
>                 /* clear wake-up sources */
>                 ret = smsc95xx_read_reg_nopm(dev, WUCSR, &val);
>                 if (ret < 0)
> @@ -1741,6 +1868,26 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
>         return skb;
>  }
>
> +static int smsc95xx_manage_power(struct usbnet *dev, int on)
> +{
> +       struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]);
> +
> +       dev->intf->needs_remote_wakeup = on;
> +
> +       if (pdata->features & FEATURE_AUTOSUSPEND)
> +               return 0;
> +
> +       /* this chip revision doesn't support autosuspend */
> +       netdev_info(dev->net, "hardware doesn't support USB autosuspend\n");

Both the comment and the log are misleading.

> +
> +       if (on)
> +               usb_autopm_get_interface_no_resume(dev->intf);
> +       else
> +               usb_autopm_put_interface(dev->intf);

As mentioned previously, playing the get/put trick is not good, why
can't we set .manage_power as null for these devices?

> +       return 0;
> +}
> +
>  static const struct driver_info smsc95xx_info = {
>         .description    = "smsc95xx USB 2.0 Ethernet",
>         .bind           = smsc95xx_bind,
> @@ -1750,6 +1897,7 @@ static const struct driver_info smsc95xx_info = {
>         .rx_fixup       = smsc95xx_rx_fixup,
>         .tx_fixup       = smsc95xx_tx_fixup,
>         .status         = smsc95xx_status,
> +       .manage_power   = smsc95xx_manage_power,
>         .flags          = FLAG_ETHER | FLAG_SEND_ZLP | FLAG_LINK_INTR,
>  };
>
> @@ -1857,6 +2005,7 @@ static struct usb_driver smsc95xx_driver = {
>         .reset_resume   = smsc95xx_resume,
>         .disconnect     = usbnet_disconnect,
>         .disable_hub_initiated_lpm = 1,
> +       .supports_autosuspend = 1,
>  };
>
>  module_usb_driver(smsc95xx_driver);
> --
> 1.7.10.4
>

Thanks,
--
Ming Lei

^ permalink raw reply

* Re: [GIT PULL net-next 01/17] ndisc: Fix size calculation for headers.
From: YOSHIFUJI Hideaki @ 2012-12-19  3:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20121218.162430.512853714678597525.davem@davemloft.net>

David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Tue, 18 Dec 2012 16:23:38 -0800 (PST)
> 
>> I'm really disappointed that the very first patch in this series is
>> buggy, and you didn't even bother to repost the absolutely required
>> "00/17" email for this series which is where you're supposed to give a
>> top-level overview of your changes as well as any GIT pull request.
> 
> Also, right now the net-next tree is not open, so these changes
> aren't even appropriate to be submitting right now.

Sorry about this, and I'll repost later.

--yoshfuji

^ permalink raw reply

* Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang
From: Joe Jin @ 2012-12-19  3:04 UTC (permalink / raw)
  To: Fujinaka, Todd
  Cc: Ethan Zhao, Ben Hutchings, Mary Mcgrath, netdev@vger.kernel.org,
	e1000-devel@lists.sf.net, linux-kernel@vger.kernel.org, linux-pci
In-Reply-To: <9B4A1B1917080E46B64F07F2989DADD638C60A9B@ORSMSX102.amr.corp.intel.com>

Hi all,

I backported mps commits and ask customer pass "pci=pcie_bus_peer2pee" to kernel
to limited MPS to 128 and issue disappeared, sound like this is a BIOS bug.

Thanks all of your help.

Best Regards,
Joe

On 11/29/12 23:52, Fujinaka, Todd wrote:
> Someone else pointed this out to me locally. If you have a non-client BIOS, you should be able to set the MaxPayloadSize using setpci. You have to make sure that you're being consistent throughout all the associated links.
> 
> Todd Fujinaka
> Technical Marketing Engineer
> LAN Access Division (LAD)
> Intel Corporation
> todd.fujinaka@intel.com
> (503) 712-4565
> 
> 
> -----Original Message-----
> From: Ethan Zhao [mailto:ethan.kernel@gmail.com] 
> Sent: Wednesday, November 28, 2012 7:10 PM
> To: Fujinaka, Todd
> Cc: Joe Jin; Ben Hutchings; Mary Mcgrath; netdev@vger.kernel.org; e1000-devel@lists.sf.net; linux-kernel@vger.kernel.org; linux-pci
> Subject: Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang
> 
> Joe,
>     Possibly your customer is running a kernel without source code on a platform whose vendor wouldn't like to fix BIOS issue( Is that a HP/Dell server ?).
>     Anyway, to see if is a payload issue or,  you could change the payload size with setpci tool to those devices and set the link retrain bit to trigger the link retraining to debug the issue and identity the root cause.  I thinks it is much easier than modify the BIOS or  eeprom of NIC.
> 
>     e.g.
>    set device control register to 0f 00   (128 bytes payload size)
>    #   setpci -v -s 00:02.0 98.w=000f
>    set device link control register to 60h (retrain the link)
>    #  setpci -v -s 00:02.0 a0.b=60
> 
>   Hope it works,  Just my 2 cents.
> 
> Ethan.zhao@oracle.com
> 
> On Wed, Nov 28, 2012 at 11:53 PM, Fujinaka, Todd <todd.fujinaka@intel.com> wrote:
>> The only EEPROM I know about or can speak to is the one attached to the 82571 and it doesn't set the MaxPayloadSize. That's done by the BIOS.
>>
>> Todd Fujinaka
>> Technical Marketing Engineer
>> LAN Access Division (LAD)
>> Intel Corporation
>> todd.fujinaka@intel.com
>> (503) 712-4565
>>
>>
>> -----Original Message-----
>> From: Joe Jin [mailto:joe.jin@oracle.com]
>> Sent: Wednesday, November 28, 2012 12:31 AM
>> To: Ben Hutchings
>> Cc: Fujinaka, Todd; Mary Mcgrath; netdev@vger.kernel.org; 
>> e1000-devel@lists.sf.net; linux-kernel@vger.kernel.org; linux-pci
>> Subject: Re: [E1000-devel] 82571EB: Detected Hardware Unit Hang
>>
>> On 11/28/12 02:10, Ben Hutchings wrote:
>>> On Tue, 2012-11-27 at 17:32 +0000, Fujinaka, Todd wrote:
>>>> Forgive me if I'm being too repetitious as I think some of this has 
>>>> been mentioned in the past.
>>>>
>>>> We (and by we I mean the Ethernet part and driver) can only change 
>>>> the advertised availability of a larger MaxPayloadSize. The size is 
>>>> negotiated by both sides of the link when the link is established.
>>>> The driver should not change the size of the link as it would be 
>>>> poking at registers outside of its scope and is controlled by the 
>>>> upstream bridge (not us).
>>> [...]
>>>
>>> MaxPayloadSize (MPS) is not negotiated between devices but is 
>>> programmed by the system firmware (at least for devices present at 
>>> boot - the kernel may be responsible in case of hotplug).  You can 
>>> use the kernel parameter 'pci=pcie_bus_perf' (or one of several 
>>> others) to set a policy that overrides this, but no policy will allow 
>>> setting MPS above the device's MaxPayloadSizeSupported (MPSS).
>>>
>>
>> Ben,
>>
>> Unfortunately I'm using 3.0.x kernel and this is not included in the kernel.
>> So I'm trying to use ethtool modify it from eeprom to see if help or no.
>>
>>
>> Todd, I'll review all MaxPayload for all devices, but need to say if it mismatch, customer could not modify it from BIOS for there was not entry at there, to test it, we have to find how to verify if this is the root cause, so still need to find the offset in eeprom.
>>
>> Thanks in advance,
>> Joe
>>


-- 
Oracle <http://www.oracle.com>
Joe Jin | Software Development Senior Manager | +8610.6106.5624
ORACLE | Linux and Virtualization
No. 24 Zhongguancun Software Park, Haidian District | 100193 Beijing 

^ 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