* Re: [PATCH 6/15] drivers/net/wireless/iwlegacy/4965-mac.c: adjust duplicate test
From: Stanislaw Gruszka @ 2013-01-21 12:07 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, John W. Linville, linux-wireless, netdev,
linux-kernel
In-Reply-To: <1358773378-4700-7-git-send-email-Julia.Lawall@lip6.fr>
On Mon, Jan 21, 2013 at 02:02:50PM +0100, Julia Lawall wrote:
> From: Julia Lawall <Julia.Lawall@lip6.fr>
>
> Delete successive tests to the same location. This looks like simple code
> duplication.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @s exists@
> local idexpression y;
> expression x,e;
> @@
>
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
> return ...; }
> ... when != \(y = e\|y += e\|y -= e\|y |= e\|y &= e\|y++\|y--\|&y\)
> when != \(XT_GETPAGE(...,y)\|WMI_CMD_BUF(...)\)
> *if ( \(x == NULL\|IS_ERR(x)\|y != 0\) )
> { ... when forall
> return ...; }
> // </smpl>
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
ACK
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Sascha Hauer @ 2013-01-21 12:06 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FD2172.2020608@grandegger.com>
On Mon, Jan 21, 2013 at 12:07:30PM +0100, Wolfgang Grandegger wrote:
> On 01/21/2013 11:07 AM, Sascha Hauer wrote:
> > On Mon, Jan 21, 2013 at 09:56:24AM +0100, Wolfgang Grandegger wrote:
> >> On 01/21/2013 09:37 AM, Sascha Hauer wrote:
> >>> There may be multiple phys on an mdio bus. This series adds support
> >>> for this to the fec driver. I recently had a board which has a switch
> >>> connected to the fec's mdio bus, so I had to pick the correct phy.
> >>
> >> Pick one PHY from a switch port? Well, does a PHY-less (or fixed-link)
> >> configuration for a switch not make more sense?
> >
> > Yes, you're probably right.
> >
> >> Various ARM Ethernet
> >> contoller drivers do not support it. I recently needed a hack for an
> >> AT91 board.
> >
> > I wonder how we want to proceed. Should there be a devicetree property
> > 'fixed-link' like done for fs_enet (and not recommended for new code,
> > stated in the comment above of_phy_connect_fixed_link)?
>
> Also the gianfar and ucc_geth drivers use this interface (via fixed
> link phy). I tried to use it for the AT91 macb driver but stopped
> quickly because the usage was not straight forward (too much code)...
> even if the idea of using a fake fixed-link phy is not bad.
>
> > Currently I have a property 'phy' in the fec binding which has a phandle
> > to a phy provided by the fec's mdio bus, but this could equally well
>
> But than the cable must be connected to the associated switch port.
>
> > point to a fixed dummy phy:
> >
> > phy = &fixed-phy;
>
> The link speed, full/half duplex and maybe some mroe parameter should
> be configurable via device tree.
Well this could be done when the fixed phy driver could be registered
with the devicetree, maybe like this:
fixed-phy: mdiophy {
compatible = "mdio-fixed-phy";
link = "100FD";
};
The good thing about this would be that every ethernet driver could just
use such a fixed phy, any external mdio phy (like on Marvell Armada) or
just a phy connected to the internal mdio interface provided by the ethernet
core.
I probably should write a RFC to devicetree-discuss.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC PATCH 3/3] xfrm4: Invalidate all ipv4 routes on IPsec pmtu events
From: Steffen Klassert @ 2013-01-21 12:04 UTC (permalink / raw)
To: David Miller; +Cc: Yurij.Plotnikov, bhutchings, netdev, Alexandra.Kossovsky
In-Reply-To: <20130118.143923.1384219845592340856.davem@davemloft.net>
On Fri, Jan 18, 2013 at 02:39:23PM -0500, David Miller wrote:
> From: Steffen Klassert <steffen.klassert@secunet.com>
> Date: Fri, 18 Jan 2013 09:16:01 +0100
>
> > On IPsec pmtu events we can't access the transport headers of
> > the original packet, so we can't find the socket that sent
> > the packet. The only chance to notify the socket about the
> > pmtu change is to force a relookup for all routes. This
> > patch implenents this for the IPsec protocols.
> >
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
>
> Expensive, but I really can't come up with a better idea.
I've just applied this to the ipsec tree.
I'll send a pull request tomorrow.
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 12:04 UTC (permalink / raw)
To: Stefan Richter; +Cc: YOSHIFUJI Hideaki, netdev, linux1394-devel, David Miller
In-Reply-To: <20130121090951.103c0680@stein>
Stefan Richter wrote:
> On Jan 20 Stephan Gatzka wrote:
>> On 01/20/2013 07:47 PM, YOSHIFUJI Hideaki wrote:
>>
>>> My current position is to change "mac address" to
>>>
>>> struct fwnet_hwaddr {
>>> u8 guid[8];
>>> u8 max_rec;
>>> u8 sspd;
>>> u8 fifo[6];
>>> };
>>>
>>
>> That is something I'm not really convinced of. As Stefan Richter pointed
>> out clearly, the fifo address might be different between IPv4 and IPv6
>> communication.
>
> If it is of any help, the initial implementation could assume that IPv4
> unicast_FIFO and IPv6 unicast_FIFO are the same. RFC 3146 is silent on
> this topic (which means it can be one way or the other), but from an
> implementation point of view, using one FIFO offset for both seems quite
> natural. Currently the only existing RFC 3146 implementation which is
> known to us is Mac OS X, and since your tests with OS X 10.6 went well,
> they obviously use one offset for both protocols.
>
> But if we actually put this assumption into the implementation now, we
> should make sure that we can easily expand the implementation later in the
> event that a third implementation comes across which uses separate
> unicast_FIFOs.
Well, FIFO for which side?
I do believe sender will not (or say, must not) care if they use
different FIFO for both protocol or not.
Assume that peer has FIFO per protocol, one for IPv4 and another for
IPv6. ARP advertise FIFO for IPv4 and NDP advertise FIFO for IPv6.
neighbour subsystem has protocol dependent tables, and two different
NCEs (neighbour cache entries) will be created. So, sender will
correctly get FIFO from NCE for each protocol.
------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122412
^ permalink raw reply
* Re: [PATCH net] xfrm: fix freed block size calculation in xfrm_policy_fini()
From: Steffen Klassert @ 2013-01-21 12:02 UTC (permalink / raw)
To: Michal Kubecek; +Cc: Herbert Xu, David S. Miller, netdev
In-Reply-To: <20130118153446.3AE05C1AFE@unicorn.suse.cz>
On Fri, Jan 18, 2013 at 04:03:48PM +0100, Michal Kubecek wrote:
> Missing multiplication of block size by sizeof(struct hlist_head)
> can cause xfrm_hash_free() to be called with wrong second argument
> so that kfree() is called on a block allocated with vzalloc() or
> __get_free_pages() or free_pages() is called with wrong order when
> a namespace with enough policies is removed.
>
> Bug introduced by commit a35f6c5d, i.e. versions >= 2.6.29 are
> affected.
>
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
Applied, Thanks!
^ permalink raw reply
* [PATCH 2/2] ipv4: Add a socket release callback for datagram sockets
From: Steffen Klassert @ 2013-01-21 12:00 UTC (permalink / raw)
To: David Miller; +Cc: Yurij M. Plotnikov, Julian Anastasov, netdev
In-Reply-To: <20130121115911.GE30530@secunet.com>
This implements a socket release callback function to check
if the socket cached route got invalid during the time
we owned the socket. The function is used from udp, raw
and ping sockets.
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/ip.h | 2 ++
net/ipv4/datagram.c | 25 +++++++++++++++++++++++++
net/ipv4/ping.c | 1 +
net/ipv4/raw.c | 1 +
net/ipv4/udp.c | 1 +
5 files changed, 30 insertions(+)
diff --git a/include/net/ip.h b/include/net/ip.h
index 0707fb9..a68f838 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -143,6 +143,8 @@ static inline struct sk_buff *ip_finish_skb(struct sock *sk, struct flowi4 *fl4)
extern int ip4_datagram_connect(struct sock *sk,
struct sockaddr *uaddr, int addr_len);
+extern void ip4_datagram_release_cb(struct sock *sk);
+
struct ip_reply_arg {
struct kvec iov[1];
int flags;
diff --git a/net/ipv4/datagram.c b/net/ipv4/datagram.c
index 424fafb..b28e863 100644
--- a/net/ipv4/datagram.c
+++ b/net/ipv4/datagram.c
@@ -85,3 +85,28 @@ out:
return err;
}
EXPORT_SYMBOL(ip4_datagram_connect);
+
+void ip4_datagram_release_cb(struct sock *sk)
+{
+ const struct inet_sock *inet = inet_sk(sk);
+ const struct ip_options_rcu *inet_opt;
+ __be32 daddr = inet->inet_daddr;
+ struct flowi4 fl4;
+ struct rtable *rt;
+
+ if (! __sk_dst_get(sk) || __sk_dst_check(sk, 0))
+ return;
+
+ rcu_read_lock();
+ inet_opt = rcu_dereference(inet->inet_opt);
+ if (inet_opt && inet_opt->opt.srr)
+ daddr = inet_opt->opt.faddr;
+ rt = ip_route_output_ports(sock_net(sk), &fl4, sk, daddr,
+ inet->inet_saddr, inet->inet_dport,
+ inet->inet_sport, sk->sk_protocol,
+ RT_CONN_FLAGS(sk), sk->sk_bound_dev_if);
+ if (!IS_ERR(rt))
+ __sk_dst_set(sk, &rt->dst);
+ rcu_read_unlock();
+}
+EXPORT_SYMBOL_GPL(ip4_datagram_release_cb);
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index 8f3d054..6f9c072 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -738,6 +738,7 @@ struct proto ping_prot = {
.recvmsg = ping_recvmsg,
.bind = ping_bind,
.backlog_rcv = ping_queue_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = ping_v4_hash,
.unhash = ping_v4_unhash,
.get_port = ping_v4_get_port,
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 73d1e4d..6f08991 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -894,6 +894,7 @@ struct proto raw_prot = {
.recvmsg = raw_recvmsg,
.bind = raw_bind,
.backlog_rcv = raw_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = raw_hash_sk,
.unhash = raw_unhash_sk,
.obj_size = sizeof(struct raw_sock),
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 79c8dbe..1f4d405 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1952,6 +1952,7 @@ struct proto udp_prot = {
.recvmsg = udp_recvmsg,
.sendpage = udp_sendpage,
.backlog_rcv = __udp_queue_rcv_skb,
+ .release_cb = ip4_datagram_release_cb,
.hash = udp_lib_hash,
.unhash = udp_lib_unhash,
.rehash = udp_v4_rehash,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 1/2] ipv4: Invalidate the socket cached route on pmtu events if possible
From: Steffen Klassert @ 2013-01-21 11:59 UTC (permalink / raw)
To: David Miller; +Cc: Yurij M. Plotnikov, Julian Anastasov, netdev
The route lookup in ipv4_sk_update_pmtu() might return a route
different from the route we cached at the socket. This is because
standart routes are per cpu, so each cpu has it's own struct rtable.
This means that we do not invalidate the socket cached route if the
NET_RX_SOFTIRQ is not served by the same cpu that the sending socket
uses. As a result, the cached route reused until we disconnect.
With this patch we invalidate the socket cached route if possible.
If the socket is owened by the user, we can't update the cached
route directly. A followup patch will implement socket release
callback functions for datagram sockets to handle this case.
Reported-by: Yurij M. Plotnikov <Yurij.Plotnikov@oktetlabs.ru>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/ipv4/route.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 259cbee..132737a 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -965,7 +965,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu,
}
EXPORT_SYMBOL_GPL(ipv4_update_pmtu);
-void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
+static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
{
const struct iphdr *iph = (const struct iphdr *) skb->data;
struct flowi4 fl4;
@@ -978,6 +978,46 @@ void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
ip_rt_put(rt);
}
}
+
+void ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
+{
+ const struct iphdr *iph = (const struct iphdr *) skb->data;
+ struct flowi4 fl4;
+ struct rtable *rt;
+ struct dst_entry *dst;
+
+ bh_lock_sock(sk);
+ rt = (struct rtable *) __sk_dst_get(sk);
+
+ if (sock_owned_by_user(sk) || !rt) {
+ __ipv4_sk_update_pmtu(skb, sk, mtu);
+ goto out;
+ }
+
+ __build_flow_key(&fl4, sk, iph, 0, 0, 0, 0, 0);
+
+ if (!__sk_dst_check(sk, 0)) {
+ rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
+ if (IS_ERR(rt))
+ goto out;
+ }
+
+ __ip_rt_update_pmtu((struct rtable *) rt->dst.path, &fl4, mtu);
+
+ dst = dst_check(&rt->dst, 0);
+ if (!dst) {
+ rt = ip_route_output_flow(sock_net(sk), &fl4, sk);
+ if (IS_ERR(rt))
+ goto out;
+
+ dst = &rt->dst;
+ }
+
+ __sk_dst_set(sk, dst);
+
+out:
+ bh_unlock_sock(sk);
+}
EXPORT_SYMBOL_GPL(ipv4_sk_update_pmtu);
void ipv4_redirect(struct sk_buff *skb, struct net *net,
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Sascha Hauer @ 2013-01-21 11:56 UTC (permalink / raw)
To: Florian Fainelli
Cc: Wolfgang Grandegger, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FD22B4.5070307@openwrt.org>
On Mon, Jan 21, 2013 at 12:12:52PM +0100, Florian Fainelli wrote:
> On 01/21/2013 12:07 PM, Wolfgang Grandegger wrote:
> >On 01/21/2013 11:07 AM, Sascha Hauer wrote:
> >>On Mon, Jan 21, 2013 at 09:56:24AM +0100, Wolfgang Grandegger wrote:
> >>>On 01/21/2013 09:37 AM, Sascha Hauer wrote:
> >>>>There may be multiple phys on an mdio bus. This series adds support
> >>>>for this to the fec driver. I recently had a board which has a switch
> >>>>connected to the fec's mdio bus, so I had to pick the correct phy.
> >>>
> >>>Pick one PHY from a switch port? Well, does a PHY-less (or fixed-link)
> >>>configuration for a switch not make more sense?
> >>
> >>Yes, you're probably right.
> >>
> >>>Various ARM Ethernet
> >>>contoller drivers do not support it. I recently needed a hack for an
> >>>AT91 board.
> >>
> >>I wonder how we want to proceed. Should there be a devicetree property
> >>'fixed-link' like done for fs_enet (and not recommended for new code,
> >>stated in the comment above of_phy_connect_fixed_link)?
> >
> >Also the gianfar and ucc_geth drivers use this interface (via fixed
> >link phy). I tried to use it for the AT91 macb driver but stopped
> >quickly because the usage was not straight forward (too much code)...
> >even if the idea of using a fake fixed-link phy is not bad.
> >
> >>Currently I have a property 'phy' in the fec binding which has a phandle
> >>to a phy provided by the fec's mdio bus, but this could equally well
> >
> >But than the cable must be connected to the associated switch port.
> >
> >>point to a fixed dummy phy:
> >>
> >> phy = &fixed-phy;
> >
> >The link speed, full/half duplex and maybe some mroe parameter should
> >be configurable via device tree.
> >
> >>Currently there seems to be no common convention for the devicetree how
> >>to handle such situations, or am I missing something?
> >
> >That's also may impression. There seem to be a few more related hacks:
> >
> >$ find . -name '*.c'| xargs grep -i "phy-less"
> >./ethernet/amd/au1000_eth.c: netdev_info(dev, "using PHY-less setup\n");
> >./ethernet/amd/au1000_eth.c: } else { /* PHY-less op, assume full-duplex */
> >./ethernet/ibm/emac/core.c: /* PHY-less configuration.
> >./ethernet/ibm/emac/core.c: /* PHY-less configuration.
> >
> >I would prefer to handle the "fixed-link" property of the ethernet dt
> >node directly in the driver with a generic helper function.
>
> Is not what of_phy_connect_fixed_link() offer? I am not sure there
> can be much done by an helper than that.
The comment above this function states:
/**
* of_phy_connect_fixed_link - Parse fixed-link property and return a dummy phy
* @dev: pointer to net_device claiming the phy
* @hndlr: Link state callback for the network device
* @iface: PHY data interface type
*
* This function is a temporary stop-gap and will be removed soon. It is
* only to support the fs_enet, ucc_geth and gianfar Ethernet drivers. Do
* not call this function from new drivers.
*/
And the commit introducing it has:
Note: the dummy phy handling in arch/powerpc is a bit of a hack and
needs to be reworked. This function is being added now to solve the
regression in the Ethernet drivers, but it should be considered a
temporary measure until the fixed link handling can be reworked.
The 'temporary measure' exists for 3 1/2 years now ;)
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [patch v3] doc: add nf_conntrack sysctl api documentation
From: Pablo Neira Ayuso @ 2013-01-21 11:52 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, rob, linux-doc, kuznet, jmorris, yoshfuji,
netfilter-devel, netfilter, coreteam, fw, Neal Murphy
In-Reply-To: <1358499908-1268-1-git-send-email-jiri@resnulli.us>
On Fri, Jan 18, 2013 at 10:05:08AM +0100, Jiri Pirko wrote:
> I grepped through the code and picked bits about nf_conntrack sysctl api
> and put that into one documentation file.
Applied, thanks a lot Jiri.
I have also included some grammar improvements that Neil Murphy has
proposed (he seems to have accidentally trimmed off the CC list).
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 11:50 UTC (permalink / raw)
To: stephan.gatzka; +Cc: linux1394-devel, netdev, David Miller, YOSHIFUJI Hideaki
In-Reply-To: <50FCDF5D.3060300@gmail.com>
Stephan Gatzka wrote:
>> And, firewire driver does not seem to support zerocopy
>> (fragmented skb) support. It help performance.
>
> O.k., good point. I will have a look into that. But I suggest to not mix up this issue with the IPv6 over firewire enhancement. That should be a separate patch.
I agree, but do we have to two fifos? for what?
If it is for performance, zerocopy should help.
--yoshfuji
^ permalink raw reply
* Re: [RFC:] struct net_device_ops: Add function pointer to fill device specific ndisc information
From: YOSHIFUJI Hideaki @ 2013-01-21 11:50 UTC (permalink / raw)
To: Stefan Richter; +Cc: stephan.gatzka, netdev, linux1394-devel, David Miller
In-Reply-To: <20130121083957.6e2c5a68@stein>
(2013年01月21日 16:39), Stefan Richter wrote:
> On Jan 21 Stephan Gatzka wrote:
>>> We could have multiple "net_device"s per single physical
>>> interface at the same time, then.
>>
>> Of course, but I would avoid it if it's not necessary. What's the
>> problem with introducing a function pointer in struct net_device or
>> struct net_device_ops?
>
> Two net_device instances on one 1394 card would be awkward: They would
> have to share one instance of isochronous reception context (for reception
> of asynchronous 1394 streams; those are used for broadcasts and
> multicasts). Such a sharing is surely possible, but if double net_device
> instantiation can be avoided, then avoid it.
>
> Not to mention the user interface problem of having two netifs, one which
> only supports IPv4 and another one which only supports IPv6. So far I
> never had IPv6 configured into a Linux kernel, but I suppose that folks
> are used to be able to use eth0 etc. for both protocols.
>
>>> Multicast is a big issue. Because IPv6 is fan of
>>> multicast, and it uses link-local multicast as its
>>> core infrastructure. Without infrastructure to
>>> support it, I'm not going to agree.
>>
>> firewire net supports multicast and we use it very often. My patch to
>> support IPv6 does not change it. In fact, because I can communicate via
>> IPv6 between two firewire nodes, multicast _is_ running. The driver
>> does not do lot's of special things with multicast packets. But
>> multicast packets are recognized because they have to be sent somehow
>> different (GASP).
>
> (At the moment we transport multicasts to the same 1394 channel like
> broadcasts.
>
> IEEE 1394 supports two addressing modes: IEEE 1212 based memory-mapable
> addresses of the form bus:node:offset = 10 bits + 6 bits + 48 bits for
> node-to-node communication, and "channel" = a 6 bit channel number for
> communication of 1 node to 0...n nodes.
>
> RFC 2734 broadcasts and multicasts use the channel addressing type. Hence
> max_rec/ sspd/ unicast_FIFO come not into play with broadcasts and
> multicasts. Broadcasts are sent to a fixed known channel number;
> multicasts are sent either to the broadcast channel number or to a
> separate channel number which is negotiated per multicast group using RFC
> 2734 Multicast Channel Allocation Protocol. For the time being the Linux
> driver only implements multicasts to the broadcast channel.)
>
Sending multicast without MCAP is okay,
but receiving multicast without MCAP is broken.
We have to be able to listen to NS (DAD) on separate
channel which is been negotiated over MCAP, for example.
--yoshfuji
^ permalink raw reply
* Re: [PATCH net-next V6 02/14] bridge: Add vlan filtering infrastructure
From: Shmulik Ladkani @ 2013-01-21 11:45 UTC (permalink / raw)
To: vyasevic, bridge
Cc: Stephen Hemminger, Michał Mirosław, netdev, davem,
shemminger, mst
In-Reply-To: <50FC9F03.5000102@redhat.com>
Hi Vlad,
On Sun, 20 Jan 2013 20:50:59 -0500 Vlad Yasevich <vyasevic@redhat.com> wrote:
> On 01/20/2013 02:38 PM, Stephen Hemminger wrote:
> > Let's assume the people that really want this feature are using a lot
> > of vlan's. i.e n = 1000 or so. A bitmap is O(1). Any hash list would
> > incur a just a big memory penalty for the list head. In other words
> > a full bitmap is 4096 bits = 512 bytes. If you use hash list,
> > then the equivalent memory size would be only 64 list heads, therefore
> > a bitmap is a better choice than a hlist.
> >
>
> This was the approach taken in the RFC v1 of this series. What I found
> was that while it worked very well as far as speed goes, it was a bit
> cumbersome to extend it to support pvids and it would completely fall
> on its face for egress policy that Shmulik is suggesting. So any kinds
> of extensions to it were tough to do.
I don't see why this is the case.
How about (sketch only, names questionable...):
struct net_bridge {
+ unsigned long vlan_port_membership_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
+ unsigned long vlan_port_egress_policy_bitmap[VLAN_N_VID][PORT_BITMAP_LEN];
}
(can be alloc'ed instead of the arrays being part of the struct)
struct net_bridge_port {
+ u16 pvid;
};
Allows O(1) to the query "is port P member of vlan V".
Allows O(1) to the query "should vlan V egress tagged/untagged on port P".
I guess this might simplify the data structures involved, avoiding the
refcounts, etc...
The penaties are:
- memory
- aesthetics (?)
- inefficient if query is "give me the entire list of VLANs port P is
member of". But do we have such a query in bridge's code?
You say it went cumbersome. Am I missing something?
BTW, altenatively, you may:
struct net_bridge_port {
+ unsigned long vlan_membership_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ unsigned long vlan_egress_policy_bitmap[BITS_TO_LONGS(VLAN_N_VID)];
+ u16 pvid;
};
Which also allows O(1) to "is port 'nbp' member of vlan V".
Difference:
- For the membership structure:
former (within net_bridge) uses 4096 * BR_MAX_PORTS bits,
latter (within net_bridge_port) uses NumOfNBPs * 4096 bits
- better aesthetics (?)
Regards,
Shmulik
^ permalink raw reply
* Re: PMTU discovery is broken on kernel 3.7.1 for UDP sockets
From: Steffen Klassert @ 2013-01-21 11:38 UTC (permalink / raw)
To: Yurij M. Plotnikov; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <50FD2708.6060906@oktetlabs.ru>
On Mon, Jan 21, 2013 at 03:31:20PM +0400, Yurij M. Plotnikov wrote:
> On 01/18/13 12:11, Steffen Klassert wrote:
> >
> >I'll send three patches in reply to this mail. Applying patch one and two
> >should fix the bug you reported. If you use IPsec you need patch three too.
> >The patches are marked as RFC, I'll do proper patch submission if you can
> >confirm that they fix this bug.
> Sorry for long delay. I've just checked patches 1+2 and they fix the
> bug for me.
>
Thanks a lot for reporting and testing!
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Wolfgang Grandegger @ 2013-01-21 11:33 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FD22B4.5070307@openwrt.org>
On 01/21/2013 12:12 PM, Florian Fainelli wrote:
> On 01/21/2013 12:07 PM, Wolfgang Grandegger wrote:
>> On 01/21/2013 11:07 AM, Sascha Hauer wrote:
>>> On Mon, Jan 21, 2013 at 09:56:24AM +0100, Wolfgang Grandegger wrote:
>>>> On 01/21/2013 09:37 AM, Sascha Hauer wrote:
>>>>> There may be multiple phys on an mdio bus. This series adds support
>>>>> for this to the fec driver. I recently had a board which has a switch
>>>>> connected to the fec's mdio bus, so I had to pick the correct phy.
>>>>
>>>> Pick one PHY from a switch port? Well, does a PHY-less (or fixed-link)
>>>> configuration for a switch not make more sense?
>>>
>>> Yes, you're probably right.
>>>
>>>> Various ARM Ethernet
>>>> contoller drivers do not support it. I recently needed a hack for an
>>>> AT91 board.
>>>
>>> I wonder how we want to proceed. Should there be a devicetree property
>>> 'fixed-link' like done for fs_enet (and not recommended for new code,
>>> stated in the comment above of_phy_connect_fixed_link)?
>>
>> Also the gianfar and ucc_geth drivers use this interface (via fixed
>> link phy). I tried to use it for the AT91 macb driver but stopped
>> quickly because the usage was not straight forward (too much code)...
>> even if the idea of using a fake fixed-link phy is not bad.
>>
>>> Currently I have a property 'phy' in the fec binding which has a phandle
>>> to a phy provided by the fec's mdio bus, but this could equally well
>>
>> But than the cable must be connected to the associated switch port.
>>
>>> point to a fixed dummy phy:
>>>
>>> phy = &fixed-phy;
>>
>> The link speed, full/half duplex and maybe some mroe parameter should
>> be configurable via device tree.
>>
>>> Currently there seems to be no common convention for the devicetree how
>>> to handle such situations, or am I missing something?
>>
>> That's also may impression. There seem to be a few more related hacks:
>>
>> $ find . -name '*.c'| xargs grep -i "phy-less"
>> ./ethernet/amd/au1000_eth.c: netdev_info(dev, "using
>> PHY-less setup\n");
>> ./ethernet/amd/au1000_eth.c: } else { /* PHY-less op, assume
>> full-duplex */
>> ./ethernet/ibm/emac/core.c: /* PHY-less configuration.
>> ./ethernet/ibm/emac/core.c: /* PHY-less configuration.
>>
>> I would prefer to handle the "fixed-link" property of the ethernet dt
>> node directly in the driver with a generic helper function.
>
> Is not what of_phy_connect_fixed_link() offer? I am not sure there can
> be much done by an helper than that.
Probably. I now remember that my primary problem with the AT91 board was
that it does not have yet device tree support.
Wolfgang.
^ permalink raw reply
* Re: [PATCH 2/2] CDC_NCM: adding support FLAG_NOARP for Infineon modem platform
From: Sergei Shtylyov @ 2013-01-21 11:33 UTC (permalink / raw)
To: Wei Shuai
Cc: dcbw-H+wXaHxf7aLQT0dZR+AlfA, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
peter-Y+HMSxxDrH8, oneukum-l3A5Bk7waGM,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
alexey.orishko-0IS4wlFg1OjSUeElwK9/Pw, bjorn-yOkvZcmFvRU,
linux-usb-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CACa7zynymtYN4OtTjVNTNdB23eNU4D-G+JqaTxjaPFekRHkRAg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello.
On 21-01-2013 6:20, Wei Shuai wrote:
> The main problem is that these devices don't support ethernet. They
> support IP (v4 and _maybe_ v6) with an ethernet header. Many of them
> will do ARP (and IPv6 ND) as well to complete the picture, but some of
> them don't and that's what these drivers try to deal with.
>
> Note that most of the devices will run a DHCP server, so there is some
> sort of IP broadcast support. Whether that qualifies as proper ethernet
> broadcast support is another question...
>
> These devices are attempting to bridge an IP-only point-to-point
> interface and an ethernet over USB interface, with the intention to make
> the point-to-point interface look like ethernet to applications and
> users. This is of course always going to be imperfect. But I believe
> that we should aim to help the firmware achive this goal when writing
> drivers instead of working against it. Setting IFF_NOARP and not
> IFF_POINTTOPOINT is one way to do that. ---- by Bjorn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
Why are you replying with this long story to my simple patch comments? :-)
> 2013/1/21, Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org>:
>> Hello.
>> On 20-01-2013 10:12, Wei Shuai wrote:
>>> Infineon(now Intel) HSPA Modem platform NCM cannot support ARP. we can
>>> define a new common structure wwan_noarp_info.
>> Wrap your lines at 76-80 columns maximum please.
>>> Then more similiar NO ARP devices can be handled easily
s/NO/no/.
>>> Signed-off-by: Wei Shuai <cpuwolf-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> drivers/net/usb/cdc_ncm.c | 21 +++++++++++++++++++++
>>> 1 files changed, 21 insertions(+), 0 deletions(-)
>>> diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
>>> index 71b6e92..2d699b6 100644
>>> --- a/drivers/net/usb/cdc_ncm.c
>>> +++ b/drivers/net/usb/cdc_ncm.c
>>> @@ -1155,6 +1155,20 @@ static const struct driver_info wwan_info = {
>>> .tx_fixup = cdc_ncm_tx_fixup,
>>> };
>>>
>>> +/* Same as wwan_info, but with IFF_NOARP */
>> FLAG_NOARP, you mean?
>>> +static const struct driver_info wwan_noarp_info = {
>>> + .description = "Mobile Broadband Network Device (NO ARP)",
>>> + .flags = FLAG_POINTTOPOINT | FLAG_NO_SETINT | FLAG_MULTI_PACKET
>>> + | FLAG_WWAN | FLAG_NOARP,
WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: PMTU discovery is broken on kernel 3.7.1 for UDP sockets
From: Yurij M. Plotnikov @ 2013-01-21 11:31 UTC (permalink / raw)
To: Steffen Klassert; +Cc: Ben Hutchings, netdev, Alexandra N. Kossovsky
In-Reply-To: <20130118081145.GB24987@secunet.com>
On 01/18/13 12:11, Steffen Klassert wrote:
> On Mon, Jan 14, 2013 at 12:26:59PM +0400, Yurij M. Plotnikov wrote:
>
>> Could you, please, tell me is there any news about this bug?
>>
>>
> Ok, I tried to reconstruct your testcase and I can confirm this bug.
> It happens only on the first pmtu event on a given route. It works
> if I run this test again. Actually it works sometimes even on the
> first pmtu event, so it took me quite a while to understand what's ging on.
>
> This is because standart routes (no pmtu or redirect events happened on
> that route) are per cpu, so each cpu has it's own struct rtable. This means
> that we do not invalidate the socket cached route if the NET_RX_SOFTIRQ
> (which handles the pmtu event) is not served by the same cpu that the
> sending socket uses. Exceptional routes (routes where a pmtu or redirect
> event occured) are not per cpu, so there is no problem.
>
> I'll send three patches in reply to this mail. Applying patch one and two
> should fix the bug you reported. If you use IPsec you need patch three too.
> The patches are marked as RFC, I'll do proper patch submission if you can
> confirm that they fix this bug.
>
Sorry for long delay. I've just checked patches 1+2 and they fix the bug
for me.
Yurij.
^ permalink raw reply
* [PATCH V6 3/3] virtio-net: reset virtqueue affinity when doing cpu hotplug
From: Wanlong Gao @ 2013-01-21 11:25 UTC (permalink / raw)
To: linux-kernel
Cc: Rusty Russell, Michael S. Tsirkin, Jason Wang, Eric Dumazet,
virtualization, netdev, Wanlong Gao
In-Reply-To: <1358767524-17934-1-git-send-email-gaowanlong@cn.fujitsu.com>
Add a cpu notifier to virtio-net, so that we can reset the
virtqueue affinity if the cpu hotplug happens. It improve
the performance through enabling or disabling the virtqueue
affinity after doing cpu hotplug.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Dumazet <erdnetdev@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
V5->V6:
deal with CPU_DOWN_PREPARE separately by just cleaning affinity
V4->V5:
New method to deal with the cpu hotplug actions (Rusty)
drivers/net/virtio_net.c | 33 +++++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 1a35a8c..1d93999 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -26,6 +26,7 @@
#include <linux/scatterlist.h>
#include <linux/if_vlan.h>
#include <linux/slab.h>
+#include <linux/cpu.h>
static int napi_weight = 128;
module_param(napi_weight, int, 0444);
@@ -126,6 +127,9 @@ struct virtnet_info {
/* Per-cpu variable to show the mapping from CPU to virtqueue */
int __percpu *vq_index;
+
+ /* CPU hot plug notifier */
+ struct notifier_block nb;
};
struct skb_vnet_hdr {
@@ -1067,6 +1071,26 @@ static void virtnet_set_affinity(struct virtnet_info *vi)
vi->affinity_hint_set = true;
}
+static int virtnet_cpu_callback(struct notifier_block *nfb,
+ unsigned long action, void *hcpu)
+{
+ struct virtnet_info *vi = container_of(nfb, struct virtnet_info, nb);
+
+ switch(action & ~CPU_TASKS_FROZEN) {
+ case CPU_ONLINE:
+ case CPU_DOWN_FAILED:
+ case CPU_DEAD:
+ virtnet_set_affinity(vi);
+ break;
+ case CPU_DOWN_PREPARE:
+ virtnet_clean_affinity(vi, (long)hcpu);
+ break;
+ default:
+ break;
+ }
+ return NOTIFY_OK;
+}
+
static void virtnet_get_ringparam(struct net_device *dev,
struct ethtool_ringparam *ring)
{
@@ -1541,6 +1565,13 @@ static int virtnet_probe(struct virtio_device *vdev)
}
}
+ vi->nb.notifier_call = &virtnet_cpu_callback;
+ err = register_hotcpu_notifier(&vi->nb);
+ if (err) {
+ pr_debug("virtio_net: registering cpu notifier failed\n");
+ goto free_recv_bufs;
+ }
+
/* Assume link up if device can't report link status,
otherwise get link status from config. */
if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
@@ -1587,6 +1618,8 @@ static void virtnet_remove(struct virtio_device *vdev)
{
struct virtnet_info *vi = vdev->priv;
+ unregister_hotcpu_notifier(&vi->nb);
+
/* Prevent config work handler from accessing the device. */
mutex_lock(&vi->config_lock);
vi->config_enable = false;
--
1.8.1
^ permalink raw reply related
* [PATCH V6 2/3] virtio-net: split out clean affinity function
From: Wanlong Gao @ 2013-01-21 11:25 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael S. Tsirkin, netdev, virtualization, Eric Dumazet
In-Reply-To: <1358767524-17934-1-git-send-email-gaowanlong@cn.fujitsu.com>
Split out the clean affinity function to virtnet_clean_affinity().
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Dumazet <erdnetdev@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
V5->V6: NEW
drivers/net/virtio_net.c | 67 +++++++++++++++++++++++++++---------------------
1 file changed, 38 insertions(+), 29 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 70cd957..1a35a8c 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1016,48 +1016,57 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
return 0;
}
-static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
+static void virtnet_clean_affinity(struct virtnet_info *vi, long hcpu)
{
int i;
int cpu;
- /* In multiqueue mode, when the number of cpu is equal to the number of
- * queue pairs, we let the queue pairs to be private to one cpu by
- * setting the affinity hint to eliminate the contention.
- */
- if ((vi->curr_queue_pairs == 1 ||
- vi->max_queue_pairs != num_online_cpus()) && set) {
- if (vi->affinity_hint_set)
- set = false;
- else
- return;
- }
-
- if (set) {
- i = 0;
- for_each_online_cpu(cpu) {
- virtqueue_set_affinity(vi->rq[i].vq, cpu);
- virtqueue_set_affinity(vi->sq[i].vq, cpu);
- *per_cpu_ptr(vi->vq_index, cpu) = i;
- i++;
- }
-
- vi->affinity_hint_set = true;
- } else {
- for(i = 0; i < vi->max_queue_pairs; i++) {
+ if (vi->affinity_hint_set) {
+ for (i = 0; i < vi->max_queue_pairs; i++) {
virtqueue_set_affinity(vi->rq[i].vq, -1);
virtqueue_set_affinity(vi->sq[i].vq, -1);
}
i = 0;
- for_each_online_cpu(cpu)
+ for_each_online_cpu(cpu) {
+ if (cpu == hcpu)
+ continue;
*per_cpu_ptr(vi->vq_index, cpu) =
++i % vi->curr_queue_pairs;
+ }
vi->affinity_hint_set = false;
}
}
+static void virtnet_set_affinity(struct virtnet_info *vi)
+{
+ int i;
+ int cpu;
+
+ /* In multiqueue mode, when the number of cpu is equal to the number of
+ * queue pairs, we let the queue pairs to be private to one cpu by
+ * setting the affinity hint to eliminate the contention.
+ */
+ if (vi->curr_queue_pairs == 1 ||
+ vi->max_queue_pairs != num_online_cpus()) {
+ if (vi->affinity_hint_set)
+ virtnet_clean_affinity(vi, -1);
+ else
+ return;
+ }
+
+ i = 0;
+ for_each_online_cpu(cpu) {
+ virtqueue_set_affinity(vi->rq[i].vq, cpu);
+ virtqueue_set_affinity(vi->sq[i].vq, cpu);
+ *per_cpu_ptr(vi->vq_index, cpu) = i;
+ i++;
+ }
+
+ vi->affinity_hint_set = true;
+}
+
static void virtnet_get_ringparam(struct net_device *dev,
struct ethtool_ringparam *ring)
{
@@ -1105,7 +1114,7 @@ static int virtnet_set_channels(struct net_device *dev,
netif_set_real_num_rx_queues(dev, queue_pairs);
get_online_cpus();
- virtnet_set_affinity(vi, true);
+ virtnet_set_affinity(vi);
put_online_cpus();
}
@@ -1274,7 +1283,7 @@ static void virtnet_del_vqs(struct virtnet_info *vi)
{
struct virtio_device *vdev = vi->vdev;
- virtnet_set_affinity(vi, false);
+ virtnet_clean_affinity(vi, -1);
vdev->config->del_vqs(vdev);
@@ -1398,7 +1407,7 @@ static int init_vqs(struct virtnet_info *vi)
goto err_free;
get_online_cpus();
- virtnet_set_affinity(vi, true);
+ virtnet_set_affinity(vi);
put_online_cpus();
return 0;
--
1.8.1
^ permalink raw reply related
* [PATCH V6 1/3] virtio-net: fix the set affinity bug when CPU IDs are not consecutive
From: Wanlong Gao @ 2013-01-21 11:25 UTC (permalink / raw)
To: linux-kernel; +Cc: Michael S. Tsirkin, netdev, virtualization, Eric Dumazet
As Michael mentioned, set affinity and select queue will not work very
well when CPU IDs are not consecutive, this can happen with hot unplug.
Fix this bug by traversal the online CPUs, and create a per cpu variable
to find the mapping from CPU to the preferable virtual-queue.
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Eric Dumazet <erdnetdev@gmail.com>
Cc: virtualization@lists.linux-foundation.org
Cc: netdev@vger.kernel.org
Signed-off-by: Wanlong Gao <gaowanlong@cn.fujitsu.com>
---
V5->V6:
remove {get|put}_online_cpus from virtnet_del_vqs (Jason)
V4->V5:
Add get/put_online_cpus to avoid CPUs go up and down during operations (Rusty)
V3->V4:
move vq_index into virtnet_info (Jason)
change the mapping value when not setting affinity (Jason)
address the comments about select_queue (Rusty)
drivers/net/virtio_net.c | 58 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 47 insertions(+), 11 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..70cd957 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -123,6 +123,9 @@ struct virtnet_info {
/* Does the affinity hint is set for virtqueues? */
bool affinity_hint_set;
+
+ /* Per-cpu variable to show the mapping from CPU to virtqueue */
+ int __percpu *vq_index;
};
struct skb_vnet_hdr {
@@ -1016,6 +1019,7 @@ static int virtnet_vlan_rx_kill_vid(struct net_device *dev, u16 vid)
static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
{
int i;
+ int cpu;
/* In multiqueue mode, when the number of cpu is equal to the number of
* queue pairs, we let the queue pairs to be private to one cpu by
@@ -1029,16 +1033,29 @@ static void virtnet_set_affinity(struct virtnet_info *vi, bool set)
return;
}
- for (i = 0; i < vi->max_queue_pairs; i++) {
- int cpu = set ? i : -1;
- virtqueue_set_affinity(vi->rq[i].vq, cpu);
- virtqueue_set_affinity(vi->sq[i].vq, cpu);
- }
+ if (set) {
+ i = 0;
+ for_each_online_cpu(cpu) {
+ virtqueue_set_affinity(vi->rq[i].vq, cpu);
+ virtqueue_set_affinity(vi->sq[i].vq, cpu);
+ *per_cpu_ptr(vi->vq_index, cpu) = i;
+ i++;
+ }
- if (set)
vi->affinity_hint_set = true;
- else
+ } else {
+ for(i = 0; i < vi->max_queue_pairs; i++) {
+ virtqueue_set_affinity(vi->rq[i].vq, -1);
+ virtqueue_set_affinity(vi->sq[i].vq, -1);
+ }
+
+ i = 0;
+ for_each_online_cpu(cpu)
+ *per_cpu_ptr(vi->vq_index, cpu) =
+ ++i % vi->curr_queue_pairs;
+
vi->affinity_hint_set = false;
+ }
}
static void virtnet_get_ringparam(struct net_device *dev,
@@ -1087,7 +1104,9 @@ static int virtnet_set_channels(struct net_device *dev,
netif_set_real_num_tx_queues(dev, queue_pairs);
netif_set_real_num_rx_queues(dev, queue_pairs);
+ get_online_cpus();
virtnet_set_affinity(vi, true);
+ put_online_cpus();
}
return err;
@@ -1127,12 +1146,19 @@ static int virtnet_change_mtu(struct net_device *dev, int new_mtu)
/* To avoid contending a lock hold by a vcpu who would exit to host, select the
* txq based on the processor id.
- * TODO: handle cpu hotplug.
*/
static u16 virtnet_select_queue(struct net_device *dev, struct sk_buff *skb)
{
- int txq = skb_rx_queue_recorded(skb) ? skb_get_rx_queue(skb) :
- smp_processor_id();
+ int txq;
+ struct virtnet_info *vi = netdev_priv(dev);
+
+ if (skb_rx_queue_recorded(skb)) {
+ txq = skb_get_rx_queue(skb);
+ } else {
+ txq = *__this_cpu_ptr(vi->vq_index);
+ if (txq == -1)
+ txq = 0;
+ }
while (unlikely(txq >= dev->real_num_tx_queues))
txq -= dev->real_num_tx_queues;
@@ -1371,7 +1397,10 @@ static int init_vqs(struct virtnet_info *vi)
if (ret)
goto err_free;
+ get_online_cpus();
virtnet_set_affinity(vi, true);
+ put_online_cpus();
+
return 0;
err_free:
@@ -1453,6 +1482,10 @@ static int virtnet_probe(struct virtio_device *vdev)
if (vi->stats == NULL)
goto free;
+ vi->vq_index = alloc_percpu(int);
+ if (vi->vq_index == NULL)
+ goto free_stats;
+
mutex_init(&vi->config_lock);
vi->config_enable = true;
INIT_WORK(&vi->config_work, virtnet_config_changed_work);
@@ -1476,7 +1509,7 @@ static int virtnet_probe(struct virtio_device *vdev)
/* Allocate/initialize the rx/tx queues, and invoke find_vqs */
err = init_vqs(vi);
if (err)
- goto free_stats;
+ goto free_index;
netif_set_real_num_tx_queues(dev, 1);
netif_set_real_num_rx_queues(dev, 1);
@@ -1520,6 +1553,8 @@ free_recv_bufs:
free_vqs:
cancel_delayed_work_sync(&vi->refill);
virtnet_del_vqs(vi);
+free_index:
+ free_percpu(vi->vq_index);
free_stats:
free_percpu(vi->stats);
free:
@@ -1554,6 +1589,7 @@ static void virtnet_remove(struct virtio_device *vdev)
flush_work(&vi->config_work);
+ free_percpu(vi->vq_index);
free_percpu(vi->stats);
free_netdev(vi->dev);
}
--
1.8.1
^ permalink raw reply related
* [PATCH v5 3/3] virtio-net: introduce a new control to set macaddr
From: Amos Kong @ 2013-01-21 11:17 UTC (permalink / raw)
To: davem, netdev; +Cc: mst, rusty, qemu-devel, kvm, virtualization
In-Reply-To: <1358767043-24548-1-git-send-email-akong@redhat.com>
Currently we write MAC address to pci config space byte by byte,
this means that we have an intermediate step where mac is wrong.
This patch introduced a new control command to set MAC address,
it's atomic.
VIRTIO_NET_F_CTRL_MAC_ADDR is a new feature bit for compatibility.
Signed-off-by: Amos Kong <akong@redhat.com>
---
drivers/net/virtio_net.c | 21 ++++++++++++++++++---
include/uapi/linux/virtio_net.h | 8 +++++++-
2 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 395ab4f..701408a 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -802,14 +802,28 @@ static int virtnet_set_mac_address(struct net_device *dev, void *p)
struct virtnet_info *vi = netdev_priv(dev);
struct virtio_device *vdev = vi->vdev;
int ret;
+ struct sockaddr *addr = p;
+ struct scatterlist sg;
- ret = eth_mac_addr(dev, p);
+ ret = eth_prepare_mac_addr_change(dev, p);
if (ret)
return ret;
- if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_MAC_ADDR)) {
+ sg_init_one(&sg, addr->sa_data, dev->addr_len);
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_MAC,
+ VIRTIO_NET_CTRL_MAC_ADDR_SET,
+ &sg, 1, 0)) {
+ dev_warn(&vdev->dev,
+ "Failed to set mac address by vq command.\n");
+ return -EINVAL;
+ }
+ } else if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
vdev->config->set(vdev, offsetof(struct virtio_net_config, mac),
- dev->dev_addr, dev->addr_len);
+ addr->sa_data, dev->addr_len);
+ }
+
+ eth_commit_mac_addr_change(dev, p);
return 0;
}
@@ -1627,6 +1641,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
+ VIRTIO_NET_F_CTRL_MAC_ADDR,
};
static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 848e358..a5a8c88 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -53,6 +53,7 @@
* network */
#define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
* Steering */
+#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
@@ -127,7 +128,7 @@ typedef __u8 virtio_net_ctrl_ack;
#define VIRTIO_NET_CTRL_RX_NOBCAST 5
/*
- * Control the MAC filter table.
+ * Control the MAC
*
* The MAC filter table is managed by the hypervisor, the guest should
* assume the size is infinite. Filtering should be considered
@@ -140,6 +141,10 @@ typedef __u8 virtio_net_ctrl_ack;
* first sg list contains unicast addresses, the second is for multicast.
* This functionality is present if the VIRTIO_NET_F_CTRL_RX feature
* is available.
+ *
+ * The ADDR_SET command requests one out scatterlist, it contains a
+ * 6 bytes MAC address. This functionality is present if the
+ * VIRTIO_NET_F_CTRL_MAC_ADDR feature is available.
*/
struct virtio_net_ctrl_mac {
__u32 entries;
@@ -148,6 +153,7 @@ struct virtio_net_ctrl_mac {
#define VIRTIO_NET_CTRL_MAC 1
#define VIRTIO_NET_CTRL_MAC_TABLE_SET 0
+ #define VIRTIO_NET_CTRL_MAC_ADDR_SET 1
/*
* Control VLAN filtering
--
1.7.1
^ permalink raw reply related
* [PATCH v5 2/3] net: split eth_mac_addr for better error handling
From: Amos Kong @ 2013-01-21 11:17 UTC (permalink / raw)
To: davem, netdev; +Cc: mst, rusty, qemu-devel, kvm, virtualization
In-Reply-To: <1358767043-24548-1-git-send-email-akong@redhat.com>
From: Stefan Hajnoczi <stefanha@gmail.com>
When we set mac address, software mac address in system and hardware mac
address all need to be updated. Current eth_mac_addr() doesn't allow
callers to implement error handling nicely.
This patch split eth_mac_addr() to prepare part and real commit part,
then we can prepare first, and try to change hardware address, then do
the real commit if hardware address is set successfully.
Signed-off-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Amos Kong <akong@redhat.com>
---
include/linux/etherdevice.h | 2 ++
net/ethernet/eth.c | 41 +++++++++++++++++++++++++++++++++++------
2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index 1a43e1b..c623861 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -40,6 +40,8 @@ extern int eth_header_cache(const struct neighbour *neigh, struct hh_cache *hh,
extern void eth_header_cache_update(struct hh_cache *hh,
const struct net_device *dev,
const unsigned char *haddr);
+extern int eth_prepare_mac_addr_change(struct net_device *dev, void *p);
+extern void eth_commit_mac_addr_change(struct net_device *dev, void *p);
extern int eth_mac_addr(struct net_device *dev, void *p);
extern int eth_change_mtu(struct net_device *dev, int new_mtu);
extern int eth_validate_addr(struct net_device *dev);
diff --git a/net/ethernet/eth.c b/net/ethernet/eth.c
index bc39c8c..a36c85e 100644
--- a/net/ethernet/eth.c
+++ b/net/ethernet/eth.c
@@ -272,6 +272,36 @@ void eth_header_cache_update(struct hh_cache *hh,
EXPORT_SYMBOL(eth_header_cache_update);
/**
+ * eth_prepare_mac_addr_change - prepare for mac change
+ * @dev: network device
+ * @p: socket address
+ */
+int eth_prepare_mac_addr_change(struct net_device *dev, void *p)
+{
+ struct sockaddr *addr = p;
+
+ if (!(dev->priv_flags & IFF_LIVE_ADDR_CHANGE) && netif_running(dev))
+ return -EBUSY;
+ if (!is_valid_ether_addr(addr->sa_data))
+ return -EADDRNOTAVAIL;
+ return 0;
+}
+EXPORT_SYMBOL(eth_prepare_mac_addr_change);
+
+/**
+ * eth_commit_mac_addr_change - commit mac change
+ * @dev: network device
+ * @p: socket address
+ */
+void eth_commit_mac_addr_change(struct net_device *dev, void *p)
+{
+ struct sockaddr *addr = p;
+
+ memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+}
+EXPORT_SYMBOL(eth_commit_mac_addr_change);
+
+/**
* eth_mac_addr - set new Ethernet hardware address
* @dev: network device
* @p: socket address
@@ -283,13 +313,12 @@ EXPORT_SYMBOL(eth_header_cache_update);
*/
int eth_mac_addr(struct net_device *dev, void *p)
{
- struct sockaddr *addr = p;
+ int ret;
- if (!(dev->priv_flags & IFF_LIVE_ADDR_CHANGE) && netif_running(dev))
- return -EBUSY;
- if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
- memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+ ret = eth_prepare_mac_addr_change(dev, p);
+ if (ret < 0)
+ return ret;
+ eth_commit_mac_addr_change(dev, p);
return 0;
}
EXPORT_SYMBOL(eth_mac_addr);
--
1.7.1
^ permalink raw reply related
* [PATCH v5 1/3] move virtnet_send_command() above virtnet_set_mac_address()
From: Amos Kong @ 2013-01-21 11:17 UTC (permalink / raw)
To: davem, netdev; +Cc: mst, rusty, qemu-devel, kvm, virtualization
In-Reply-To: <1358767043-24548-1-git-send-email-akong@redhat.com>
We want to send vq command to set mac address in
virtnet_set_mac_address(), so do this function moving.
Fixed a little issue of coding style.
Signed-off-by: Amos Kong <akong@redhat.com>
---
drivers/net/virtio_net.c | 89 ++++++++++++++++++++++-----------------------
1 files changed, 44 insertions(+), 45 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index a6fcf15..395ab4f 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -753,6 +753,50 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
return NETDEV_TX_OK;
}
+/*
+ * Send command via the control virtqueue and check status. Commands
+ * supported by the hypervisor, as indicated by feature bits, should
+ * never fail unless improperly formated.
+ */
+static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
+ struct scatterlist *data, int out, int in)
+{
+ struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
+ struct virtio_net_ctrl_hdr ctrl;
+ virtio_net_ctrl_ack status = ~0;
+ unsigned int tmp;
+ int i;
+
+ /* Caller should know better */
+ BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
+ (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
+
+ out++; /* Add header */
+ in++; /* Add return status */
+
+ ctrl.class = class;
+ ctrl.cmd = cmd;
+
+ sg_init_table(sg, out + in);
+
+ sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
+ for_each_sg(data, s, out + in - 2, i)
+ sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
+ sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
+
+ BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
+
+ virtqueue_kick(vi->cvq);
+
+ /* Spin for a response, the kick causes an ioport write, trapping
+ * into the hypervisor, so the request should be handled immediately.
+ */
+ while (!virtqueue_get_buf(vi->cvq, &tmp))
+ cpu_relax();
+
+ return status == VIRTIO_NET_OK;
+}
+
static int virtnet_set_mac_address(struct net_device *dev, void *p)
{
struct virtnet_info *vi = netdev_priv(dev);
@@ -819,51 +863,6 @@ static void virtnet_netpoll(struct net_device *dev)
}
#endif
-/*
- * Send command via the control virtqueue and check status. Commands
- * supported by the hypervisor, as indicated by feature bits, should
- * never fail unless improperly formated.
- */
-static bool virtnet_send_command(struct virtnet_info *vi, u8 class, u8 cmd,
- struct scatterlist *data, int out, int in)
-{
- struct scatterlist *s, sg[VIRTNET_SEND_COMMAND_SG_MAX + 2];
- struct virtio_net_ctrl_hdr ctrl;
- virtio_net_ctrl_ack status = ~0;
- unsigned int tmp;
- int i;
-
- /* Caller should know better */
- BUG_ON(!virtio_has_feature(vi->vdev, VIRTIO_NET_F_CTRL_VQ) ||
- (out + in > VIRTNET_SEND_COMMAND_SG_MAX));
-
- out++; /* Add header */
- in++; /* Add return status */
-
- ctrl.class = class;
- ctrl.cmd = cmd;
-
- sg_init_table(sg, out + in);
-
- sg_set_buf(&sg[0], &ctrl, sizeof(ctrl));
- for_each_sg(data, s, out + in - 2, i)
- sg_set_buf(&sg[i + 1], sg_virt(s), s->length);
- sg_set_buf(&sg[out + in - 1], &status, sizeof(status));
-
- BUG_ON(virtqueue_add_buf(vi->cvq, sg, out, in, vi, GFP_ATOMIC) < 0);
-
- virtqueue_kick(vi->cvq);
-
- /*
- * Spin for a response, the kick causes an ioport write, trapping
- * into the hypervisor, so the request should be handled immediately.
- */
- while (!virtqueue_get_buf(vi->cvq, &tmp))
- cpu_relax();
-
- return status == VIRTIO_NET_OK;
-}
-
static void virtnet_ack_link_announce(struct virtnet_info *vi)
{
rtnl_lock();
--
1.7.1
^ permalink raw reply related
* [PATCH v5 0/3] make mac programming for virtio net more robust
From: Amos Kong @ 2013-01-21 11:17 UTC (permalink / raw)
To: davem, netdev; +Cc: virtualization, qemu-devel, kvm, mst
Currenly mac is programmed byte by byte. This means that we
have an intermediate step where mac is wrong.
Third patch introduced a new vq control command to set mac
address, it's atomic.
V2: check return of sending command, delay eth_mac_addr()
V3: restore software address when fail to set hardware address
V4: split eth_mac_addr, fix error handle
V5: rebase patches to net-next tree
Amos Kong (2):
move virtnet_send_command() above virtnet_set_mac_address()
virtio-net: introduce a new control to set macaddr
Stefan Hajnoczi (1):
net: split eth_mac_addr for better error handling
drivers/net/virtio_net.c | 110 ++++++++++++++++++++++-----------------
include/linux/etherdevice.h | 2 +
include/uapi/linux/virtio_net.h | 8 +++-
net/ethernet/eth.c | 41 ++++++++++++--
4 files changed, 106 insertions(+), 55 deletions(-)
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Florian Fainelli @ 2013-01-21 11:12 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: Sascha Hauer, netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <50FD2172.2020608@grandegger.com>
On 01/21/2013 12:07 PM, Wolfgang Grandegger wrote:
> On 01/21/2013 11:07 AM, Sascha Hauer wrote:
>> On Mon, Jan 21, 2013 at 09:56:24AM +0100, Wolfgang Grandegger wrote:
>>> On 01/21/2013 09:37 AM, Sascha Hauer wrote:
>>>> There may be multiple phys on an mdio bus. This series adds support
>>>> for this to the fec driver. I recently had a board which has a switch
>>>> connected to the fec's mdio bus, so I had to pick the correct phy.
>>>
>>> Pick one PHY from a switch port? Well, does a PHY-less (or fixed-link)
>>> configuration for a switch not make more sense?
>>
>> Yes, you're probably right.
>>
>>> Various ARM Ethernet
>>> contoller drivers do not support it. I recently needed a hack for an
>>> AT91 board.
>>
>> I wonder how we want to proceed. Should there be a devicetree property
>> 'fixed-link' like done for fs_enet (and not recommended for new code,
>> stated in the comment above of_phy_connect_fixed_link)?
>
> Also the gianfar and ucc_geth drivers use this interface (via fixed
> link phy). I tried to use it for the AT91 macb driver but stopped
> quickly because the usage was not straight forward (too much code)...
> even if the idea of using a fake fixed-link phy is not bad.
>
>> Currently I have a property 'phy' in the fec binding which has a phandle
>> to a phy provided by the fec's mdio bus, but this could equally well
>
> But than the cable must be connected to the associated switch port.
>
>> point to a fixed dummy phy:
>>
>> phy = &fixed-phy;
>
> The link speed, full/half duplex and maybe some mroe parameter should
> be configurable via device tree.
>
>> Currently there seems to be no common convention for the devicetree how
>> to handle such situations, or am I missing something?
>
> That's also may impression. There seem to be a few more related hacks:
>
> $ find . -name '*.c'| xargs grep -i "phy-less"
> ./ethernet/amd/au1000_eth.c: netdev_info(dev, "using PHY-less setup\n");
> ./ethernet/amd/au1000_eth.c: } else { /* PHY-less op, assume full-duplex */
> ./ethernet/ibm/emac/core.c: /* PHY-less configuration.
> ./ethernet/ibm/emac/core.c: /* PHY-less configuration.
>
> I would prefer to handle the "fixed-link" property of the ethernet dt
> node directly in the driver with a generic helper function.
Is not what of_phy_connect_fixed_link() offer? I am not sure there can
be much done by an helper than that.
--
Florian
^ permalink raw reply
* Re: [PATCH] net: fec: Add support for multiple phys on mdiobus
From: Wolfgang Grandegger @ 2013-01-21 11:07 UTC (permalink / raw)
To: Sascha Hauer; +Cc: netdev, linux-arm-kernel, shawn.guo, davem
In-Reply-To: <20130121100701.GX1906@pengutronix.de>
On 01/21/2013 11:07 AM, Sascha Hauer wrote:
> On Mon, Jan 21, 2013 at 09:56:24AM +0100, Wolfgang Grandegger wrote:
>> On 01/21/2013 09:37 AM, Sascha Hauer wrote:
>>> There may be multiple phys on an mdio bus. This series adds support
>>> for this to the fec driver. I recently had a board which has a switch
>>> connected to the fec's mdio bus, so I had to pick the correct phy.
>>
>> Pick one PHY from a switch port? Well, does a PHY-less (or fixed-link)
>> configuration for a switch not make more sense?
>
> Yes, you're probably right.
>
>> Various ARM Ethernet
>> contoller drivers do not support it. I recently needed a hack for an
>> AT91 board.
>
> I wonder how we want to proceed. Should there be a devicetree property
> 'fixed-link' like done for fs_enet (and not recommended for new code,
> stated in the comment above of_phy_connect_fixed_link)?
Also the gianfar and ucc_geth drivers use this interface (via fixed
link phy). I tried to use it for the AT91 macb driver but stopped
quickly because the usage was not straight forward (too much code)...
even if the idea of using a fake fixed-link phy is not bad.
> Currently I have a property 'phy' in the fec binding which has a phandle
> to a phy provided by the fec's mdio bus, but this could equally well
But than the cable must be connected to the associated switch port.
> point to a fixed dummy phy:
>
> phy = &fixed-phy;
The link speed, full/half duplex and maybe some mroe parameter should
be configurable via device tree.
> Currently there seems to be no common convention for the devicetree how
> to handle such situations, or am I missing something?
That's also may impression. There seem to be a few more related hacks:
$ find . -name '*.c'| xargs grep -i "phy-less"
./ethernet/amd/au1000_eth.c: netdev_info(dev, "using PHY-less setup\n");
./ethernet/amd/au1000_eth.c: } else { /* PHY-less op, assume full-duplex */
./ethernet/ibm/emac/core.c: /* PHY-less configuration.
./ethernet/ibm/emac/core.c: /* PHY-less configuration.
I would prefer to handle the "fixed-link" property of the ethernet dt
node directly in the driver with a generic helper function.
Wolfgang.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox