* Re: bonding can't change to another slave if you ifdown the active slave
From: WANG Cong @ 2011-03-08 12:51 UTC (permalink / raw)
To: netdev
In-Reply-To: <4D75D244.9070409@gmail.com>
On Tue, 08 Mar 2011 14:52:52 +0800, Weiping Pan wrote:
> ok, I use "Host-only mode" and get
>
>
>
> an conclusion, that if the first enslaved nic is pulled out, bonding
> can't handle well.
>
> First test.
> I first enslave eth6, then pull it out, bonding doesn't work. on host,
...
>
>
> Second test
> I first enslave eth6, then pull eth7 out, bonding works well. on guest,
Can you show me your /proc/net/bonding/bond0 before and after pulling down
eth6 or eth7? And what does `ip link show` say?
^ permalink raw reply
* Re: [Patch] bonding: fix netpoll in active-backup mode
From: Neil Horman @ 2011-03-08 13:26 UTC (permalink / raw)
To: Cong Wang
Cc: linux-kernel, Jay Vosburgh, David S. Miller, Herbert Xu,
Paul E. McKenney, John W. Linville, Eric Dumazet, netdev
In-Reply-To: <4D75AD50.7060400@redhat.com>
On Tue, Mar 08, 2011 at 12:15:12PM +0800, Cong Wang wrote:
> 于 2011年03月08日 02:50, Neil Horman 写道:
> >On Mon, Mar 07, 2011 at 10:11:50PM +0800, Amerigo Wang wrote:
> >>netconsole doesn't work in active-backup mode, because we don't do anything
> >>for nic failover in active-backup mode. This patch fixes the problem by:
> >>
> >>1) make slave_enable_netpoll() and slave_disable_netpoll() callable in softirq
> >> context, that is, moving code after synchronize_rcu_bh() into call_rcu_bh()
> >> callback function, teaching kzalloc() to use GFP_ATOMIC.
> >>
> >>2) disable netpoll on old slave and enable netpoll on the new slave.
> >>
> >>Tested by ifdown the current active slave and ifup it again for several times,
> >>netconsole works well.
> >>
> >>Signed-off-by: WANG Cong<amwang@redhat.com>
> >>
> >I may be missing soething but this seems way over-complicated to me. I presume
> >the problem is that in active backup mode a failover results in the new active
> >slave not having netpoll setup on it? If thats the case, why not just setup
> >netpoll on all slaves when ndo_netpoll_setup is called on the bonding interface?
> >I don't see anything immeidately catastrophic that would happen as a result.
>
>
> But we still need to clean up the netpoll on the failing slave, which still
> needs to call slave_disable_netpoll() in monitor code, I see no big differences
> with the solution I take.
>
Why? I understand you want to free up that memory, but I don't see any special
state codified in that structure that can't wait until you disable netpoll on
the bond as a whole. Save yourself the time and trouble, enable netpoll on both
slaves when its enabled on the bond, and tear it down when its torn down on the
bond. Do worry about doing anything during a failover.
Neil
>
> >And then you wouldn't have to worry about disabling/enabling anything on a
> >failover (or during a panic for that matter). As for the rcu bits? Why are
> >they needed? One would presume that wouldn't (or at least shouldn't) be able to
> >teardown our netpoll setup until such time as all the pending frames for that
> >netpoll client have been transmitted. If we're not blocknig on that RCU isn't
> >really going to help. Seems like the proper fix is take a reference to the
> >appropriate npinfo struct in netpoll_send_skb, and drop it from the skbs
> >destructor or some such.
>
> I saw a "scheduling while in atomic" warning without touching the rcu bits.
>
> Thanks!
> --
> 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 net-next-2.6] net: reinject arps into bonding slave instead of master
From: Andy Gospodarek @ 2011-03-08 13:42 UTC (permalink / raw)
To: Jiri Pirko
Cc: Andy Gospodarek, Nicolas de Pesloüan, netdev, davem,
shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110308071350.GA2826@psychotron.redhat.com>
On Tue, Mar 08, 2011 at 08:13:51AM +0100, Jiri Pirko wrote:
> Mon, Mar 07, 2011 at 11:43:38PM CET, andy@greyhouse.net wrote:
[...]
> >
> >This patch doesn't work.
> >
> >My setup has bond0.100 -> bond0 -> eth2 and eth3. ARP monitoring is
> >enabled as is arp_valiate.
> >
> >The initial problem was just that just before vlan_on_bond_hook is
> >called, skb->dev = bond0.100 and orig_dev = eth2. (This is after
> >running goto another_route and having been called back through
> >__netif_receive_skb since vlan_hwaccel_do_receive it true.)
> >
> >Now vlan_on_bond_hook is called and we have 2 skbs.
> >
> >The original skb still have skb->dev = bond0.100 and orig_dev = eth2.
> >Since bond_arp_rcv is registered for traffic only to bond0, the handler
> >is not hit and the frame is dropped (or processed by another handler).
> >
> >The cloned skb has skb->dev = bond0 and is put back on the receive queue
> >and comes back through __netif_receive_skb. This frame will match the
> >ptype entry for bond_arp_rcv, but since orig_dev = bond0 in this case,
> >the code in bond_arp_rcv will not handle the frame.
> >
> >If we truly want to track the original interface that received the
> >frame, the following is a better option. With the recursive nature of
> >__netif_receive_skb at this point, we should really consider setting
> >orig_dev from skb_iif rather than just from skb->dev.
> >
> >diff --git a/net/core/dev.c b/net/core/dev.c
> >index 30440e7..500fdbc 100644
> >--- a/net/core/dev.c
> >+++ b/net/core/dev.c
> >@@ -3135,7 +3135,6 @@ static int __netif_receive_skb(struct sk_buff *skb)
> >
> > if (!skb->skb_iif)
> > skb->skb_iif = skb->dev->ifindex;
> >- orig_dev = skb->dev;
> >
> > skb_reset_network_header(skb);
> > skb_reset_transport_header(skb);
> >@@ -3145,6 +3144,7 @@ static int __netif_receive_skb(struct sk_buff *skb)
> >
> > rcu_read_lock();
> >
> >+ orig_dev = dev_get_by_index_rcu(dev_net(skb->dev),skb->skb_iif);
> > another_round:
> >
> > __this_cpu_inc(softnet_data.processed);
> >
>
> This was proposed earlier. people did not like this very much :(
>
Interesting. I'll have to look back at the discussion.
> I forgot to include crucial part of "goto another_round for vlan".
> Following patch should work (will test it once I get to work):
>
> Subject: [patch net-next 2.6] net: reinject arps into bonding slave instead of master
>
> Recent patch "bonding: move processing of recv handlers into
> handle_frame()" caused a regression on following net scheme:
>
> eth0 - bond0 - bond0.5
>
> where arp monitoring is happening over vlan. This patch fixes it by
> reinjecting the arp packet into bonding slave device so the bonding
> rx_handler can pickup and process it.
>
> also instead of calling __netif_receive_skb recursively, "goto another
> round" does this recursion. The point is the orig_dev variable remains
> intact.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> ---
> net/core/dev.c | 11 +++++------
> 1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c71bd18..ec330e1 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3094,12 +3094,12 @@ void netdev_rx_handler_unregister(struct net_device *dev)
> }
> EXPORT_SYMBOL_GPL(netdev_rx_handler_unregister);
>
> -static void vlan_on_bond_hook(struct sk_buff *skb)
> +static void vlan_on_bond_hook(struct sk_buff *skb, struct net_device *orig_dev)
> {
> /*
> * Make sure ARP frames received on VLAN interfaces stacked on
> * bonding interfaces still make their way to any base bonding
> - * device that may have registered for a specific ptype.
> + * device by reinjecting the frame into bonding slave (orig_dev)
> */
> if (skb->dev->priv_flags & IFF_802_1Q_VLAN &&
> vlan_dev_real_dev(skb->dev)->priv_flags & IFF_BONDING &&
> @@ -3108,7 +3108,7 @@ static void vlan_on_bond_hook(struct sk_buff *skb)
>
> if (!skb2)
> return;
> - skb2->dev = vlan_dev_real_dev(skb->dev);
> + skb2->dev = orig_dev;
> netif_rx(skb2);
> }
>
I'm pretty sure this patch will have the same catastrophic problem your
last one did. By cloning and setting skb2->dev = orig_dev you just
inserted a frame identical to the one we received right back into the
stack. It only took a few minutes for my box to melt as one frame on
the wire will cause an infinite number of frames to be received by the
stack.
> @@ -3196,13 +3196,12 @@ ncls:
> pt_prev = NULL;
> }
> if (vlan_hwaccel_do_receive(&skb)) {
> - ret = __netif_receive_skb(skb);
> - goto out;
> + goto another_round;
> } else if (unlikely(!skb))
> goto out;
> }
>
> - vlan_on_bond_hook(skb);
> + vlan_on_bond_hook(skb, orig_dev);
>
> /* deliver only exact match when indicated */
> null_or_dev = deliver_exact ? skb->dev : NULL;
^ permalink raw reply
* Re: [Patch V2] bonding: fix netpoll in active-backup mode
From: Neil Horman @ 2011-03-08 13:43 UTC (permalink / raw)
To: Amerigo Wang; +Cc: linux-kernel, Jay Vosburgh, netdev
In-Reply-To: <1299578336-5888-1-git-send-email-amwang@redhat.com>
On Tue, Mar 08, 2011 at 05:58:56PM +0800, Amerigo Wang wrote:
> V2: avoid calling slave_diable_netpoll() with write_lock_bh() held.
>
> netconsole doesn't work in active-backup mode, because we don't do anything
> for nic failover in active-backup mode. We should disable netpoll on the
> failing slave when it is detected down and enable netpoll when it becomes
> the active slave.
>
You still haven't explained why it needs to be this way. what exactly is the
shortcomming with leaving netpoll enabled on all slaves, regardless of state?
It should be sufficient if, during a failover in a mode where the inactive slave
should not tx data, that you simply clear the slave __LINK_STATE_START bit. That will
prevent higher layers from sending any queued data without you needing to muck
about with netpoll state.
Neil
> Tested by ifdown the current active slave and ifup it again for several times,
> netconsole works well.
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
>
> ---
>
> drivers/net/bonding/bond_main.c | 236 +++++++++++++++++++++------------------
> 1 files changed, 125 insertions(+), 111 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 0592e6d..102a558 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -907,6 +907,120 @@ static void bond_mc_list_flush(struct net_device *bond_dev,
> }
> }
>
> +/*--------------------------- Netpoll code ---------------------------*/
> +#ifdef CONFIG_NET_POLL_CONTROLLER
> +static inline int slave_enable_netpoll(struct slave *slave)
> +{
> + struct netpoll *np;
> + int err = 0;
> +
> + if (slave->np)
> + return 0;
> +
> + np = kzalloc(sizeof(*np), GFP_KERNEL);
> + err = -ENOMEM;
> + if (!np)
> + goto out;
> +
> + np->dev = slave->dev;
> + err = __netpoll_setup(np);
> + if (err) {
> + kfree(np);
> + goto out;
> + }
> + slave->np = np;
> +out:
> + return err;
> +}
> +static inline void slave_disable_netpoll(struct slave *slave)
> +{
> + struct netpoll *np = slave->np;
> +
> + if (!np)
> + return;
> +
> + slave->np = NULL;
> + synchronize_rcu_bh();
> + __netpoll_cleanup(np);
> + kfree(np);
> +}
> +static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
> +{
> + if (slave_dev->priv_flags & IFF_DISABLE_NETPOLL)
> + return false;
> + if (!slave_dev->netdev_ops->ndo_poll_controller)
> + return false;
> + return true;
> +}
> +
> +static void bond_poll_controller(struct net_device *bond_dev)
> +{
> +}
> +
> +static void __bond_netpoll_cleanup(struct bonding *bond)
> +{
> + struct slave *slave;
> + int i;
> +
> + bond_for_each_slave(bond, slave, i)
> + if (IS_UP(slave->dev))
> + slave_disable_netpoll(slave);
> +}
> +static void bond_netpoll_cleanup(struct net_device *bond_dev)
> +{
> + struct bonding *bond = netdev_priv(bond_dev);
> +
> + read_lock(&bond->lock);
> + __bond_netpoll_cleanup(bond);
> + read_unlock(&bond->lock);
> +}
> +
> +static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> +{
> + struct bonding *bond = netdev_priv(dev);
> + struct slave *slave;
> + int i, err = 0;
> +
> + read_lock(&bond->lock);
> + bond_for_each_slave(bond, slave, i) {
> + if (!IS_UP(slave->dev))
> + continue;
> + err = slave_enable_netpoll(slave);
> + if (err) {
> + __bond_netpoll_cleanup(bond);
> + break;
> + }
> + }
> + read_unlock(&bond->lock);
> + return err;
> +}
> +
> +static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> +{
> + return bond->dev->npinfo;
> +}
> +
> +#else
> +static inline int slave_enable_netpoll(struct slave *slave)
> +{
> + return 0;
> +}
> +static inline void slave_disable_netpoll(struct slave *slave)
> +{
> +}
> +static void bond_netpoll_cleanup(struct net_device *bond_dev)
> +{
> +}
> +static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> +{
> + return 0;
> +}
> +static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> +{
> + return NULL;
> +}
> +#endif
> +
> /*--------------------------- Active slave change ---------------------------*/
>
> /*
> @@ -1159,6 +1273,7 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> bond_set_slave_inactive_flags(old_active);
>
> if (new_active) {
> + struct netpoll_info *ni;
> bond_set_slave_active_flags(new_active);
>
> if (bond->params.fail_over_mac)
> @@ -1174,6 +1289,13 @@ void bond_change_active_slave(struct bonding *bond, struct slave *new_active)
> }
>
> write_unlock_bh(&bond->curr_slave_lock);
> +
> + ni = bond_netpoll_info(bond);
> + if (ni) {
> + new_active->dev->npinfo = ni;
> + slave_enable_netpoll(new_active);
> + }
> +
> read_unlock(&bond->lock);
>
> netdev_bonding_change(bond->dev, NETDEV_BONDING_FAILOVER);
> @@ -1280,116 +1402,6 @@ static void bond_detach_slave(struct bonding *bond, struct slave *slave)
> bond->slave_cnt--;
> }
>
> -#ifdef CONFIG_NET_POLL_CONTROLLER
> -static inline int slave_enable_netpoll(struct slave *slave)
> -{
> - struct netpoll *np;
> - int err = 0;
> -
> - np = kzalloc(sizeof(*np), GFP_KERNEL);
> - err = -ENOMEM;
> - if (!np)
> - goto out;
> -
> - np->dev = slave->dev;
> - err = __netpoll_setup(np);
> - if (err) {
> - kfree(np);
> - goto out;
> - }
> - slave->np = np;
> -out:
> - return err;
> -}
> -static inline void slave_disable_netpoll(struct slave *slave)
> -{
> - struct netpoll *np = slave->np;
> -
> - if (!np)
> - return;
> -
> - slave->np = NULL;
> - synchronize_rcu_bh();
> - __netpoll_cleanup(np);
> - kfree(np);
> -}
> -static inline bool slave_dev_support_netpoll(struct net_device *slave_dev)
> -{
> - if (slave_dev->priv_flags & IFF_DISABLE_NETPOLL)
> - return false;
> - if (!slave_dev->netdev_ops->ndo_poll_controller)
> - return false;
> - return true;
> -}
> -
> -static void bond_poll_controller(struct net_device *bond_dev)
> -{
> -}
> -
> -static void __bond_netpoll_cleanup(struct bonding *bond)
> -{
> - struct slave *slave;
> - int i;
> -
> - bond_for_each_slave(bond, slave, i)
> - if (IS_UP(slave->dev))
> - slave_disable_netpoll(slave);
> -}
> -static void bond_netpoll_cleanup(struct net_device *bond_dev)
> -{
> - struct bonding *bond = netdev_priv(bond_dev);
> -
> - read_lock(&bond->lock);
> - __bond_netpoll_cleanup(bond);
> - read_unlock(&bond->lock);
> -}
> -
> -static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> -{
> - struct bonding *bond = netdev_priv(dev);
> - struct slave *slave;
> - int i, err = 0;
> -
> - read_lock(&bond->lock);
> - bond_for_each_slave(bond, slave, i) {
> - if (!IS_UP(slave->dev))
> - continue;
> - err = slave_enable_netpoll(slave);
> - if (err) {
> - __bond_netpoll_cleanup(bond);
> - break;
> - }
> - }
> - read_unlock(&bond->lock);
> - return err;
> -}
> -
> -static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> -{
> - return bond->dev->npinfo;
> -}
> -
> -#else
> -static inline int slave_enable_netpoll(struct slave *slave)
> -{
> - return 0;
> -}
> -static inline void slave_disable_netpoll(struct slave *slave)
> -{
> -}
> -static void bond_netpoll_cleanup(struct net_device *bond_dev)
> -{
> -}
> -static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
> -{
> - return 0;
> -}
> -static struct netpoll_info *bond_netpoll_info(struct bonding *bond)
> -{
> - return NULL;
> -}
> -#endif
> -
> /*---------------------------------- IOCTL ----------------------------------*/
>
> static int bond_sethwaddr(struct net_device *bond_dev,
> @@ -2532,8 +2544,10 @@ static void bond_miimon_commit(struct bonding *bond)
> bond_alb_handle_link_change(bond, slave,
> BOND_LINK_DOWN);
>
> - if (slave == bond->curr_active_slave)
> + if (slave == bond->curr_active_slave) {
> + slave_disable_netpoll(slave);
> goto do_failover;
> + }
>
> continue;
>
> --
> 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
* [RFCv3] [PATCH 0/8] net-next: Phonet fixes and cleanup
From: Rémi Denis-Courmont @ 2011-03-08 13:56 UTC (permalink / raw)
To: netdev
Changes are available in the git repository at:
git://git.remlab.net/linux-phonet.git master
Rémi Denis-Courmont (8):
Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
Phonet: return an error when skb TX fails
Phonet: correct pipe backlog callback return values
Phonet: factor common code to send control messages
Phonet: allocate sock from accept syscall rather than soft IRQ
Phonet: provide pipe socket option to retrieve the pipe identifier
Phonet: support active connection without pipe controller on modem
Phonet: kill the ST-Ericsson pipe controller Kconfig
Documentation/networking/phonet.txt | 67 ++---
include/linux/phonet.h | 4 +-
include/net/phonet/pep.h | 1 -
net/phonet/Kconfig | 12 -
net/phonet/af_phonet.c | 13 +-
net/phonet/pep.c | 682 ++++++++++++++---------------------
net/phonet/socket.c | 112 +++----
7 files changed, 352 insertions(+), 539 deletions(-)
--
Rémi Denis-Courmont
http://www.remlab.net/
^ permalink raw reply
* [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: Rémi Denis-Courmont @ 2011-03-08 13:56 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/af_phonet.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 30cc676..4706b77 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -262,10 +262,9 @@ int pn_skb_send(struct sock *sk, struct sk_buff *skb,
else if (phonet_address_lookup(net, daddr) == 0) {
dev = phonet_device_get(net);
skb->pkt_type = PACKET_LOOPBACK;
- } else if (pn_sockaddr_get_object(target) == 0) {
+ } else if (dst == 0) {
/* Resource routing (small race until phonet_rcv()) */
- struct sock *sk = pn_find_sock_by_res(net,
- target->spn_resource);
+ struct sock *sk = pn_find_sock_by_res(net, res);
if (sk) {
sock_put(sk);
dev = phonet_device_get(net);
--
1.7.1
^ permalink raw reply related
* [PATCH 2/8] Phonet: return an error when skb TX fails
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/af_phonet.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/net/phonet/af_phonet.c b/net/phonet/af_phonet.c
index 4706b77..c6fffd9 100644
--- a/net/phonet/af_phonet.c
+++ b/net/phonet/af_phonet.c
@@ -195,11 +195,7 @@ static int pn_send(struct sk_buff *skb, struct net_device *dev,
if (skb->pkt_type == PACKET_LOOPBACK) {
skb_reset_mac_header(skb);
skb_orphan(skb);
- if (irq)
- netif_rx(skb);
- else
- netif_rx_ni(skb);
- err = 0;
+ err = (irq ? netif_rx(skb) : netif_rx_ni(skb)) ? -ENOBUFS : 0;
} else {
err = dev_hard_header(skb, dev, ntohs(skb->protocol),
NULL, NULL, skb->len);
@@ -208,6 +204,8 @@ static int pn_send(struct sk_buff *skb, struct net_device *dev,
goto drop;
}
err = dev_queue_xmit(skb);
+ if (unlikely(err > 0))
+ err = net_xmit_errno(err);
}
return err;
--
1.7.1
^ permalink raw reply related
* [PATCH 5/8] Phonet: allocate sock from accept syscall rather than soft IRQ
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
This moves most of the accept logic to process context, and simplifies
the code a bit at the same time.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
include/net/phonet/pep.h | 1 -
net/phonet/pep.c | 284 +++++++++++++++++++---------------------------
net/phonet/socket.c | 10 +-
3 files changed, 121 insertions(+), 174 deletions(-)
diff --git a/include/net/phonet/pep.h b/include/net/phonet/pep.h
index 38eed1b..b669fe6 100644
--- a/include/net/phonet/pep.h
+++ b/include/net/phonet/pep.h
@@ -28,7 +28,6 @@ struct pep_sock {
/* XXX: union-ify listening vs connected stuff ? */
/* Listening socket stuff: */
- struct hlist_head ackq;
struct hlist_head hlist;
/* Connected socket stuff: */
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 610794a..c0fab4c 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -42,7 +42,7 @@
* TCP_ESTABLISHED connected pipe in enabled state
*
* pep_sock locking:
- * - sk_state, ackq, hlist: sock lock needed
+ * - sk_state, hlist: sock lock needed
* - listener: read only
* - pipe_handle: read only
*/
@@ -202,11 +202,12 @@ static int pep_accept_conn(struct sock *sk, struct sk_buff *skb)
GFP_KERNEL);
}
-static int pep_reject_conn(struct sock *sk, struct sk_buff *skb, u8 code)
+static int pep_reject_conn(struct sock *sk, struct sk_buff *skb, u8 code,
+ gfp_t priority)
{
static const u8 data[4] = { PAD, PAD, PAD, 0 /* sub-blocks */ };
WARN_ON(code == PN_PIPE_NO_ERROR);
- return pep_reply(sk, skb, code, data, sizeof(data), GFP_ATOMIC);
+ return pep_reply(sk, skb, code, data, sizeof(data), priority);
}
/* Control requests are not sent by the pipe service and have a specific
@@ -365,7 +366,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
switch (hdr->message_id) {
case PNS_PEP_CONNECT_REQ:
- pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE, GFP_ATOMIC);
break;
case PNS_PEP_DISCONNECT_REQ:
@@ -574,7 +575,6 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
sk->sk_state = TCP_SYN_RECV;
sk->sk_backlog_rcv = pipe_do_rcv;
- sk->sk_destruct = pipe_destruct;
pn->rx_credits = 0;
sk->sk_state_change(sk);
@@ -582,96 +582,6 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
}
#endif
-static int pep_connreq_rcv(struct sock *sk, struct sk_buff *skb)
-{
- struct sock *newsk;
- struct pep_sock *newpn, *pn = pep_sk(sk);
- struct pnpipehdr *hdr;
- struct sockaddr_pn dst, src;
- u16 peer_type;
- u8 pipe_handle, enabled, n_sb;
- u8 aligned = 0;
-
- if (!pskb_pull(skb, sizeof(*hdr) + 4))
- return -EINVAL;
-
- hdr = pnp_hdr(skb);
- pipe_handle = hdr->pipe_handle;
- switch (hdr->state_after_connect) {
- case PN_PIPE_DISABLE:
- enabled = 0;
- break;
- case PN_PIPE_ENABLE:
- enabled = 1;
- break;
- default:
- pep_reject_conn(sk, skb, PN_PIPE_ERR_INVALID_PARAM);
- return -EINVAL;
- }
- peer_type = hdr->other_pep_type << 8;
-
- /* Parse sub-blocks (options) */
- n_sb = hdr->data[4];
- while (n_sb > 0) {
- u8 type, buf[1], len = sizeof(buf);
- const u8 *data = pep_get_sb(skb, &type, &len, buf);
-
- if (data == NULL)
- return -EINVAL;
- switch (type) {
- case PN_PIPE_SB_CONNECT_REQ_PEP_SUB_TYPE:
- if (len < 1)
- return -EINVAL;
- peer_type = (peer_type & 0xff00) | data[0];
- break;
- case PN_PIPE_SB_ALIGNED_DATA:
- aligned = data[0] != 0;
- break;
- }
- n_sb--;
- }
-
- skb = skb_clone(skb, GFP_ATOMIC);
- if (!skb)
- return -ENOMEM;
-
- /* Create a new to-be-accepted sock */
- newsk = sk_alloc(sock_net(sk), PF_PHONET, GFP_ATOMIC, sk->sk_prot);
- if (!newsk) {
- kfree_skb(skb);
- return -ENOMEM;
- }
- sock_init_data(NULL, newsk);
- newsk->sk_state = TCP_SYN_RECV;
- newsk->sk_backlog_rcv = pipe_do_rcv;
- newsk->sk_protocol = sk->sk_protocol;
- newsk->sk_destruct = pipe_destruct;
-
- newpn = pep_sk(newsk);
- pn_skb_get_dst_sockaddr(skb, &dst);
- pn_skb_get_src_sockaddr(skb, &src);
- newpn->pn_sk.sobject = pn_sockaddr_get_object(&dst);
- newpn->pn_sk.dobject = pn_sockaddr_get_object(&src);
- newpn->pn_sk.resource = pn_sockaddr_get_resource(&dst);
- skb_queue_head_init(&newpn->ctrlreq_queue);
- newpn->pipe_handle = pipe_handle;
- atomic_set(&newpn->tx_credits, 0);
- newpn->peer_type = peer_type;
- newpn->rx_credits = 0;
- newpn->rx_fc = newpn->tx_fc = PN_LEGACY_FLOW_CONTROL;
- newpn->init_enable = enabled;
- newpn->aligned = aligned;
-
- BUG_ON(!skb_queue_empty(&newsk->sk_receive_queue));
- skb_queue_head(&newsk->sk_receive_queue, skb);
- if (!sock_flag(sk, SOCK_DEAD))
- sk->sk_data_ready(sk, 0);
-
- sk_acceptq_added(sk);
- sk_add_node(newsk, &pn->ackq);
- return 0;
-}
-
/* Listening sock must be locked */
static struct sock *pep_find_pipe(const struct hlist_head *hlist,
const struct sockaddr_pn *dst,
@@ -726,22 +636,18 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sknode)
return sk_receive_skb(sknode, skb, 1);
- /* Look for a pipe handle pending accept */
- sknode = pep_find_pipe(&pn->ackq, &dst, pipe_handle);
- if (sknode) {
- sock_put(sknode);
- if (net_ratelimit())
- printk(KERN_WARNING"Phonet unconnected PEP ignored");
- goto drop;
- }
-
switch (hdr->message_id) {
case PNS_PEP_CONNECT_REQ:
- if (sk->sk_state == TCP_LISTEN && !sk_acceptq_is_full(sk))
- pep_connreq_rcv(sk, skb);
- else
- pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
- break;
+ if (sk->sk_state != TCP_LISTEN || sk_acceptq_is_full(sk)) {
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE,
+ GFP_ATOMIC);
+ break;
+ }
+ skb_queue_head(&sk->sk_receive_queue, skb);
+ sk_acceptq_added(sk);
+ if (!sock_flag(sk, SOCK_DEAD))
+ sk->sk_data_ready(sk, 0);
+ return NET_RX_SUCCESS;
#ifdef CONFIG_PHONET_PIPECTRLR
case PNS_PEP_CONNECT_RESP:
@@ -799,24 +705,16 @@ static void pep_sock_close(struct sock *sk, long timeout)
sk_common_release(sk);
lock_sock(sk);
- if (sk->sk_state == TCP_LISTEN) {
- /* Destroy the listen queue */
- struct sock *sknode;
- struct hlist_node *p, *n;
-
- sk_for_each_safe(sknode, p, n, &pn->ackq)
- sk_del_node_init(sknode);
- sk->sk_state = TCP_CLOSE;
- } else if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
+ if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
#ifndef CONFIG_PHONET_PIPECTRLR
/* Forcefully remove dangling Phonet pipe */
pipe_do_remove(sk);
#else
/* send pep disconnect request */
pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
- sk->sk_state = TCP_CLOSE;
#endif
}
+ sk->sk_state = TCP_CLOSE;
ifindex = pn->ifindex;
pn->ifindex = 0;
@@ -827,69 +725,121 @@ static void pep_sock_close(struct sock *sk, long timeout)
sock_put(sk);
}
-static int pep_wait_connreq(struct sock *sk, int noblock)
+static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp)
{
- struct task_struct *tsk = current;
- struct pep_sock *pn = pep_sk(sk);
- long timeo = sock_rcvtimeo(sk, noblock);
-
- for (;;) {
- DEFINE_WAIT(wait);
+ struct pep_sock *pn = pep_sk(sk), *newpn;
+ struct sock *newsk = NULL;
+ struct sk_buff *skb;
+ struct pnpipehdr *hdr;
+ struct sockaddr_pn dst, src;
+ int err;
+ u16 peer_type;
+ u8 pipe_handle, enabled, n_sb;
+ u8 aligned = 0;
- if (sk->sk_state != TCP_LISTEN)
- return -EINVAL;
- if (!hlist_empty(&pn->ackq))
- break;
- if (!timeo)
- return -EWOULDBLOCK;
- if (signal_pending(tsk))
- return sock_intr_errno(timeo);
+ skb = skb_recv_datagram(sk, 0, flags & O_NONBLOCK, errp);
+ if (!skb)
+ return NULL;
- prepare_to_wait_exclusive(sk_sleep(sk), &wait,
- TASK_INTERRUPTIBLE);
- release_sock(sk);
- timeo = schedule_timeout(timeo);
- lock_sock(sk);
- finish_wait(sk_sleep(sk), &wait);
+ lock_sock(sk);
+ if (sk->sk_state != TCP_LISTEN) {
+ err = -EINVAL;
+ goto drop;
}
+ sk_acceptq_removed(sk);
- return 0;
-}
+ err = -EPROTO;
+ if (!pskb_may_pull(skb, sizeof(*hdr) + 4))
+ goto drop;
-static struct sock *pep_sock_accept(struct sock *sk, int flags, int *errp)
-{
- struct pep_sock *pn = pep_sk(sk);
- struct sock *newsk = NULL;
- struct sk_buff *oskb;
- int err;
+ hdr = pnp_hdr(skb);
+ pipe_handle = hdr->pipe_handle;
+ switch (hdr->state_after_connect) {
+ case PN_PIPE_DISABLE:
+ enabled = 0;
+ break;
+ case PN_PIPE_ENABLE:
+ enabled = 1;
+ break;
+ default:
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_INVALID_PARAM,
+ GFP_KERNEL);
+ goto drop;
+ }
+ peer_type = hdr->other_pep_type << 8;
- lock_sock(sk);
- err = pep_wait_connreq(sk, flags & O_NONBLOCK);
- if (err)
- goto out;
+ /* Parse sub-blocks (options) */
+ n_sb = hdr->data[4];
+ while (n_sb > 0) {
+ u8 type, buf[1], len = sizeof(buf);
+ const u8 *data = pep_get_sb(skb, &type, &len, buf);
- newsk = __sk_head(&pn->ackq);
+ if (data == NULL)
+ goto drop;
+ switch (type) {
+ case PN_PIPE_SB_CONNECT_REQ_PEP_SUB_TYPE:
+ if (len < 1)
+ goto drop;
+ peer_type = (peer_type & 0xff00) | data[0];
+ break;
+ case PN_PIPE_SB_ALIGNED_DATA:
+ aligned = data[0] != 0;
+ break;
+ }
+ n_sb--;
+ }
- oskb = skb_dequeue(&newsk->sk_receive_queue);
- err = pep_accept_conn(newsk, oskb);
- if (err) {
- skb_queue_head(&newsk->sk_receive_queue, oskb);
+ /* Check for duplicate pipe handle */
+ newsk = pep_find_pipe(&pn->hlist, &dst, pipe_handle);
+ if (unlikely(newsk)) {
+ __sock_put(newsk);
newsk = NULL;
- goto out;
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE, GFP_KERNEL);
+ goto drop;
+ }
+
+ /* Create a new to-be-accepted sock */
+ newsk = sk_alloc(sock_net(sk), PF_PHONET, GFP_KERNEL, sk->sk_prot);
+ if (!newsk) {
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_OVERLOAD, GFP_KERNEL);
+ err = -ENOBUFS;
+ goto drop;
}
- kfree_skb(oskb);
+ sock_init_data(NULL, newsk);
+ newsk->sk_state = TCP_SYN_RECV;
+ newsk->sk_backlog_rcv = pipe_do_rcv;
+ newsk->sk_protocol = sk->sk_protocol;
+ newsk->sk_destruct = pipe_destruct;
+
+ newpn = pep_sk(newsk);
+ pn_skb_get_dst_sockaddr(skb, &dst);
+ pn_skb_get_src_sockaddr(skb, &src);
+ newpn->pn_sk.sobject = pn_sockaddr_get_object(&dst);
+ newpn->pn_sk.dobject = pn_sockaddr_get_object(&src);
+ newpn->pn_sk.resource = pn_sockaddr_get_resource(&dst);
sock_hold(sk);
- pep_sk(newsk)->listener = sk;
+ newpn->listener = sk;
+ skb_queue_head_init(&newpn->ctrlreq_queue);
+ newpn->pipe_handle = pipe_handle;
+ atomic_set(&newpn->tx_credits, 0);
+ newpn->ifindex = 0;
+ newpn->peer_type = peer_type;
+ newpn->rx_credits = 0;
+ newpn->rx_fc = newpn->tx_fc = PN_LEGACY_FLOW_CONTROL;
+ newpn->init_enable = enabled;
+ newpn->aligned = aligned;
- sock_hold(newsk);
- sk_del_node_init(newsk);
- sk_acceptq_removed(sk);
+ err = pep_accept_conn(newsk, skb);
+ if (err) {
+ sock_put(newsk);
+ newsk = NULL;
+ goto drop;
+ }
sk_add_node(newsk, &pn->hlist);
- __sock_put(newsk);
-
-out:
+drop:
release_sock(sk);
+ kfree_skb(skb);
*errp = err;
return newsk;
}
@@ -937,7 +887,7 @@ static int pep_init(struct sock *sk)
{
struct pep_sock *pn = pep_sk(sk);
- INIT_HLIST_HEAD(&pn->ackq);
+ sk->sk_destruct = pipe_destruct;
INIT_HLIST_HEAD(&pn->hlist);
skb_queue_head_init(&pn->ctrlreq_queue);
pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 65a0333..1eccfc3 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -327,6 +327,9 @@ static int pn_socket_accept(struct socket *sock, struct socket *newsock,
struct sock *newsk;
int err;
+ if (unlikely(sk->sk_state != TCP_LISTEN))
+ return -EINVAL;
+
newsk = sk->sk_prot->accept(sk, flags, &err);
if (!newsk)
return err;
@@ -363,13 +366,8 @@ static unsigned int pn_socket_poll(struct file *file, struct socket *sock,
poll_wait(file, sk_sleep(sk), wait);
- switch (sk->sk_state) {
- case TCP_LISTEN:
- return hlist_empty(&pn->ackq) ? 0 : POLLIN;
- case TCP_CLOSE:
+ if (sk->sk_state == TCP_CLOSE)
return POLLERR;
- }
-
if (!skb_queue_empty(&sk->sk_receive_queue))
mask |= POLLIN | POLLRDNORM;
if (!skb_queue_empty(&pn->ctrlreq_queue))
--
1.7.1
^ permalink raw reply related
* [PATCH 6/8] Phonet: provide pipe socket option to retrieve the pipe identifier
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
User-space sometimes needs this information.
This replaces the buggy code in CONFIG_PHONET_PIPECTRLR case:
this was never used and error cases not handled correctly.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
Documentation/networking/phonet.txt | 7 ++++---
include/linux/phonet.h | 2 +-
net/phonet/pep.c | 15 +++++++--------
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index 24ad2ad..cacac96 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -181,6 +181,10 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
interface index of the network interface created by PNPIPE_ENCAP,
or zero if encapsulation is off.
+ PNPIPE_HANDLE is a read-only integer value. It contains the underlying
+ identifier ("pipe handle") of the pipe. This is only defined for
+ socket descriptors that are already connected or being connected.
+
Phonet Pipe-controller Implementation
-------------------------------------
@@ -199,9 +203,6 @@ between itself and a remote pipe-end point (e.g. modem).
The implementation adds socket options at SOL_PNPIPE level:
- PNPIPE_PIPE_HANDLE
- It accepts an integer argument for setting value of pipe handle.
-
PNPIPE_ENABLE accepts one integer value (int). If set to zero, the pipe
is disabled. If the value is non-zero, the pipe is enabled. If the pipe
is not (yet) connected, ENOTCONN is error is returned.
diff --git a/include/linux/phonet.h b/include/linux/phonet.h
index 26c8df7..32a0965 100644
--- a/include/linux/phonet.h
+++ b/include/linux/phonet.h
@@ -36,7 +36,7 @@
/* Socket options for SOL_PNPIPE level */
#define PNPIPE_ENCAP 1
#define PNPIPE_IFINDEX 2
-#define PNPIPE_PIPE_HANDLE 3
+#define PNPIPE_HANDLE 3
#define PNPIPE_ENABLE 4
/* unused slot */
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index c0fab4c..abfb795 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -853,6 +853,7 @@ static int pep_sock_connect(struct sock *sk, struct sockaddr *addr, int len)
pn->pn_sk.dobject = pn_sockaddr_get_object(spn);
pn->pn_sk.resource = pn_sockaddr_get_resource(spn);
+ pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
PN_PIPE_DISABLE, data, 4);
}
@@ -909,14 +910,6 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
lock_sock(sk);
switch (optname) {
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNPIPE_PIPE_HANDLE:
- if (val) {
- pn->pipe_handle = val;
- break;
- }
-#endif
-
case PNPIPE_ENCAP:
if (val && val != PNPIPE_ENCAP_IP) {
err = -EINVAL;
@@ -982,6 +975,12 @@ static int pep_getsockopt(struct sock *sk, int level, int optname,
val = pn->ifindex;
break;
+ case PNPIPE_HANDLE:
+ val = pn->pipe_handle;
+ if (val == PN_PIPE_INVALID_HANDLE)
+ return -EINVAL;
+ break;
+
#ifdef CONFIG_PHONET_PIPECTRLR
case PNPIPE_ENABLE:
val = sk->sk_state == TCP_ESTABLISHED;
--
1.7.1
^ permalink raw reply related
* [PATCH 4/8] Phonet: factor common code to send control messages
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep.c | 225 ++++++++++++++++++------------------------------------
1 files changed, 73 insertions(+), 152 deletions(-)
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 40952c7..610794a 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -77,24 +77,34 @@ static unsigned char *pep_get_sb(struct sk_buff *skb, u8 *ptype, u8 *plen,
return data;
}
-static int pep_reply(struct sock *sk, struct sk_buff *oskb,
- u8 code, const void *data, int len, gfp_t priority)
+static struct sk_buff *pep_alloc_skb(struct sock *sk, const void *payload,
+ int len, gfp_t priority)
+{
+ struct sk_buff *skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+ if (!skb)
+ return NULL;
+ skb_set_owner_w(skb, sk);
+
+ skb_reserve(skb, MAX_PNPIPE_HEADER);
+ __skb_put(skb, len);
+ skb_copy_to_linear_data(skb, payload, len);
+ __skb_push(skb, sizeof(struct pnpipehdr));
+ skb_reset_transport_header(skb);
+ return skb;
+}
+
+static int pep_reply(struct sock *sk, struct sk_buff *oskb, u8 code,
+ const void *data, int len, gfp_t priority)
{
const struct pnpipehdr *oph = pnp_hdr(oskb);
struct pnpipehdr *ph;
struct sk_buff *skb;
struct sockaddr_pn peer;
- skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+ skb = pep_alloc_skb(sk, data, len, priority);
if (!skb)
return -ENOMEM;
- skb_set_owner_w(skb, sk);
- skb_reserve(skb, MAX_PNPIPE_HEADER);
- __skb_put(skb, len);
- skb_copy_to_linear_data(skb, data, len);
- __skb_push(skb, sizeof(*ph));
- skb_reset_transport_header(skb);
ph = pnp_hdr(skb);
ph->utid = oph->utid;
ph->message_id = oph->message_id + 1; /* REQ -> RESP */
@@ -105,135 +115,69 @@ static int pep_reply(struct sock *sk, struct sk_buff *oskb,
return pn_skb_send(sk, skb, &peer);
}
-#define PAD 0x00
-
-#ifdef CONFIG_PHONET_PIPECTRLR
-static int pipe_handler_send_req(struct sock *sk, u8 msg_id, gfp_t priority)
+static int pep_indicate(struct sock *sk, u8 id, u8 code,
+ const void *data, int len, gfp_t priority)
{
- int len;
+ struct pep_sock *pn = pep_sk(sk);
struct pnpipehdr *ph;
struct sk_buff *skb;
- struct pep_sock *pn = pep_sk(sk);
-
- static const u8 data[4] = {
- PAD, PAD, PAD, PAD,
- };
- switch (msg_id) {
- case PNS_PEP_CONNECT_REQ:
- len = sizeof(data);
- break;
-
- case PNS_PEP_DISCONNECT_REQ:
- case PNS_PEP_ENABLE_REQ:
- case PNS_PEP_DISABLE_REQ:
- len = 0;
- break;
-
- default:
- return -EINVAL;
- }
-
- skb = alloc_skb(MAX_PNPIPE_HEADER + len, priority);
+ skb = pep_alloc_skb(sk, data, len, priority);
if (!skb)
return -ENOMEM;
- skb_set_owner_w(skb, sk);
- skb_reserve(skb, MAX_PNPIPE_HEADER);
- if (len) {
- __skb_put(skb, len);
- skb_copy_to_linear_data(skb, data, len);
- }
- __skb_push(skb, sizeof(*ph));
- skb_reset_transport_header(skb);
ph = pnp_hdr(skb);
- ph->utid = msg_id; /* whatever */
- ph->message_id = msg_id;
+ ph->utid = 0;
+ ph->message_id = id;
ph->pipe_handle = pn->pipe_handle;
- ph->error_code = PN_PIPE_NO_ERROR;
-
+ ph->data[0] = code;
return pn_skb_send(sk, skb, NULL);
}
-static int pipe_handler_send_created_ind(struct sock *sk, u8 msg_id)
+#define PAD 0x00
+
+#ifdef CONFIG_PHONET_PIPECTRLR
+static int pipe_handler_request(struct sock *sk, u8 id, u8 code,
+ const void *data, int len)
{
- int err_code;
+ struct pep_sock *pn = pep_sk(sk);
struct pnpipehdr *ph;
struct sk_buff *skb;
- struct pep_sock *pn = pep_sk(sk);
- static u8 data[4] = {
- 0x03, 0x04,
- };
- data[2] = pn->tx_fc;
- data[3] = pn->rx_fc;
-
- /*
- * actually, below is number of sub-blocks and not error code.
- * Pipe_created_ind message format does not have any
- * error code field. However, the Phonet stack will always send
- * an error code as part of pnpipehdr. So, use that err_code to
- * specify the number of sub-blocks.
- */
- err_code = 0x01;
-
- skb = alloc_skb(MAX_PNPIPE_HEADER + sizeof(data), GFP_ATOMIC);
+ skb = pep_alloc_skb(sk, data, len, GFP_KERNEL);
if (!skb)
return -ENOMEM;
- skb_set_owner_w(skb, sk);
- skb_reserve(skb, MAX_PNPIPE_HEADER);
- __skb_put(skb, sizeof(data));
- skb_copy_to_linear_data(skb, data, sizeof(data));
- __skb_push(skb, sizeof(*ph));
- skb_reset_transport_header(skb);
ph = pnp_hdr(skb);
- ph->utid = 0;
- ph->message_id = msg_id;
+ ph->utid = id; /* whatever */
+ ph->message_id = id;
ph->pipe_handle = pn->pipe_handle;
- ph->error_code = err_code;
-
+ ph->data[0] = code;
return pn_skb_send(sk, skb, NULL);
}
-static int pipe_handler_send_ind(struct sock *sk, u8 msg_id)
+static int pipe_handler_send_created_ind(struct sock *sk)
{
- int err_code;
- struct pnpipehdr *ph;
- struct sk_buff *skb;
struct pep_sock *pn = pep_sk(sk);
+ u8 data[4] = {
+ PN_PIPE_SB_NEGOTIATED_FC, pep_sb_size(2),
+ pn->tx_fc, pn->rx_fc,
+ };
- /*
- * actually, below is a filler.
- * Pipe_enabled/disabled_ind message format does not have any
- * error code field. However, the Phonet stack will always send
- * an error code as part of pnpipehdr. So, use that err_code to
- * specify the filler value.
- */
- err_code = 0x0;
-
- skb = alloc_skb(MAX_PNPIPE_HEADER, GFP_ATOMIC);
- if (!skb)
- return -ENOMEM;
- skb_set_owner_w(skb, sk);
-
- skb_reserve(skb, MAX_PNPIPE_HEADER);
- __skb_push(skb, sizeof(*ph));
- skb_reset_transport_header(skb);
- ph = pnp_hdr(skb);
- ph->utid = 0;
- ph->message_id = msg_id;
- ph->pipe_handle = pn->pipe_handle;
- ph->error_code = err_code;
+ return pep_indicate(sk, PNS_PIPE_CREATED_IND, 1 /* sub-blocks */,
+ data, 4, GFP_ATOMIC);
+}
- return pn_skb_send(sk, skb, NULL);
+static int pipe_handler_send_ind(struct sock *sk, u8 id)
+{
+ return pep_indicate(sk, id, PAD, NULL, 0, GFP_ATOMIC);
}
static int pipe_handler_enable_pipe(struct sock *sk, int enable)
{
u8 id = enable ? PNS_PEP_ENABLE_REQ : PNS_PEP_DISABLE_REQ;
- return pipe_handler_send_req(sk, id, GFP_KERNEL);
+ return pipe_handler_request(sk, id, PAD, NULL, 0);
}
#endif
@@ -274,23 +218,21 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
struct sk_buff *skb;
struct pnpipehdr *ph;
struct sockaddr_pn dst;
+ u8 data[4] = {
+ oph->data[0], /* PEP type */
+ code, /* error code, at an unusual offset */
+ PAD, PAD,
+ };
- skb = alloc_skb(MAX_PNPIPE_HEADER + 4, priority);
+ skb = pep_alloc_skb(sk, data, 4, priority);
if (!skb)
return -ENOMEM;
- skb_set_owner_w(skb, sk);
-
- skb_reserve(skb, MAX_PHONET_HEADER);
- ph = (struct pnpipehdr *)skb_put(skb, sizeof(*ph) + 4);
+ ph = pnp_hdr(skb);
ph->utid = oph->utid;
ph->message_id = PNS_PEP_CTRL_RESP;
ph->pipe_handle = oph->pipe_handle;
ph->data[0] = oph->data[1]; /* CTRL id */
- ph->data[1] = oph->data[0]; /* PEP type */
- ph->data[2] = code; /* error code, at an usual offset */
- ph->data[3] = PAD;
- ph->data[4] = PAD;
pn_skb_get_src_sockaddr(oskb, &dst);
return pn_skb_send(sk, skb, &dst);
@@ -298,34 +240,15 @@ static int pep_ctrlreq_error(struct sock *sk, struct sk_buff *oskb, u8 code,
static int pipe_snd_status(struct sock *sk, u8 type, u8 status, gfp_t priority)
{
- struct pep_sock *pn = pep_sk(sk);
- struct pnpipehdr *ph;
- struct sk_buff *skb;
+ u8 data[4] = { type, PAD, PAD, status };
- skb = alloc_skb(MAX_PNPIPE_HEADER + 4, priority);
- if (!skb)
- return -ENOMEM;
- skb_set_owner_w(skb, sk);
-
- skb_reserve(skb, MAX_PNPIPE_HEADER + 4);
- __skb_push(skb, sizeof(*ph) + 4);
- skb_reset_transport_header(skb);
- ph = pnp_hdr(skb);
- ph->utid = 0;
- ph->message_id = PNS_PEP_STATUS_IND;
- ph->pipe_handle = pn->pipe_handle;
- ph->pep_type = PN_PEP_TYPE_COMMON;
- ph->data[1] = type;
- ph->data[2] = PAD;
- ph->data[3] = PAD;
- ph->data[4] = status;
-
- return pn_skb_send(sk, skb, NULL);
+ return pep_indicate(sk, PNS_PEP_STATUS_IND, PN_PEP_TYPE_COMMON,
+ data, 4, priority);
}
/* Send our RX flow control information to the sender.
* Socket must be locked. */
-static void pipe_grant_credits(struct sock *sk)
+static void pipe_grant_credits(struct sock *sk, gfp_t priority)
{
struct pep_sock *pn = pep_sk(sk);
@@ -335,16 +258,16 @@ static void pipe_grant_credits(struct sock *sk)
case PN_LEGACY_FLOW_CONTROL: /* TODO */
break;
case PN_ONE_CREDIT_FLOW_CONTROL:
- pipe_snd_status(sk, PN_PEP_IND_FLOW_CONTROL,
- PEP_IND_READY, GFP_ATOMIC);
- pn->rx_credits = 1;
+ if (pipe_snd_status(sk, PN_PEP_IND_FLOW_CONTROL,
+ PEP_IND_READY, priority) == 0)
+ pn->rx_credits = 1;
break;
case PN_MULTI_CREDIT_FLOW_CONTROL:
if ((pn->rx_credits + CREDITS_THR) > CREDITS_MAX)
break;
if (pipe_snd_status(sk, PN_PEP_IND_ID_MCFC_GRANT_CREDITS,
CREDITS_MAX - pn->rx_credits,
- GFP_ATOMIC) == 0)
+ priority) == 0)
pn->rx_credits = CREDITS_MAX;
break;
}
@@ -474,7 +397,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_ESTABLISHED)
break; /* Nothing to do */
sk->sk_state = TCP_ESTABLISHED;
- pipe_grant_credits(sk);
+ pipe_grant_credits(sk, GFP_ATOMIC);
break;
#endif
@@ -561,7 +484,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
if (sk->sk_state == TCP_ESTABLISHED)
break; /* Nothing to do */
sk->sk_state = TCP_ESTABLISHED;
- pipe_grant_credits(sk);
+ pipe_grant_credits(sk, GFP_ATOMIC);
break;
case PNS_PIPE_DISABLED_IND:
@@ -655,7 +578,7 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
pn->rx_credits = 0;
sk->sk_state_change(sk);
- return pipe_handler_send_created_ind(sk, PNS_PIPE_CREATED_IND);
+ return pipe_handler_send_created_ind(sk);
}
#endif
@@ -853,19 +776,15 @@ static int pipe_do_remove(struct sock *sk)
struct pnpipehdr *ph;
struct sk_buff *skb;
- skb = alloc_skb(MAX_PNPIPE_HEADER, GFP_KERNEL);
+ skb = pep_alloc_skb(sk, NULL, 0, GFP_KERNEL);
if (!skb)
return -ENOMEM;
- skb_reserve(skb, MAX_PNPIPE_HEADER);
- __skb_push(skb, sizeof(*ph));
- skb_reset_transport_header(skb);
ph = pnp_hdr(skb);
ph->utid = 0;
ph->message_id = PNS_PIPE_REMOVE_REQ;
ph->pipe_handle = pn->pipe_handle;
ph->data[0] = PAD;
-
return pn_skb_send(sk, skb, NULL);
}
#endif
@@ -894,7 +813,7 @@ static void pep_sock_close(struct sock *sk, long timeout)
pipe_do_remove(sk);
#else
/* send pep disconnect request */
- pipe_handler_send_req(sk, PNS_PEP_DISCONNECT_REQ, GFP_KERNEL);
+ pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
sk->sk_state = TCP_CLOSE;
#endif
}
@@ -980,10 +899,12 @@ static int pep_sock_connect(struct sock *sk, struct sockaddr *addr, int len)
{
struct pep_sock *pn = pep_sk(sk);
const struct sockaddr_pn *spn = (struct sockaddr_pn *)addr;
+ u8 data[4] = { 0 /* sub-blocks */, PAD, PAD, PAD };
pn->pn_sk.dobject = pn_sockaddr_get_object(spn);
pn->pn_sk.resource = pn_sockaddr_get_resource(spn);
- return pipe_handler_send_req(sk, PNS_PEP_CONNECT_REQ, GFP_KERNEL);
+ return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
+ PN_PIPE_DISABLE, data, 4);
}
#endif
@@ -1280,7 +1201,7 @@ struct sk_buff *pep_read(struct sock *sk)
struct sk_buff *skb = skb_dequeue(&sk->sk_receive_queue);
if (sk->sk_state == TCP_ESTABLISHED)
- pipe_grant_credits(sk);
+ pipe_grant_credits(sk, GFP_ATOMIC);
return skb;
}
@@ -1325,7 +1246,7 @@ static int pep_recvmsg(struct kiocb *iocb, struct sock *sk,
}
if (sk->sk_state == TCP_ESTABLISHED)
- pipe_grant_credits(sk);
+ pipe_grant_credits(sk, GFP_KERNEL);
release_sock(sk);
copy:
msg->msg_flags |= MSG_EOR;
--
1.7.1
^ permalink raw reply related
* [PATCH 3/8] Phonet: correct pipe backlog callback return values
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
In some cases, the Phonet pipe backlog callbacks returned negative
errno instead. In some other cases, NET_RX_DROP was returned even
though there was no buffering issue.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
net/phonet/pep.c | 25 +++++++++++--------------
1 files changed, 11 insertions(+), 14 deletions(-)
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 875e86c..40952c7 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -522,7 +522,8 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
if (!pn_flow_safe(pn->rx_fc)) {
err = sock_queue_rcv_skb(sk, skb);
if (!err)
- return 0;
+ return NET_RX_SUCCESS;
+ err = -ENOBUFS;
break;
}
@@ -575,7 +576,7 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
}
out:
kfree_skb(skb);
- return err;
+ return (err == -ENOBUFS) ? NET_RX_DROP : NET_RX_SUCCESS;
queue:
skb->dev = NULL;
@@ -584,7 +585,7 @@ queue:
skb_queue_tail(queue, skb);
if (!sock_flag(sk, SOCK_DEAD))
sk->sk_data_ready(sk, err);
- return 0;
+ return NET_RX_SUCCESS;
}
/* Destroy connected sock. */
@@ -686,11 +687,6 @@ static int pep_connreq_rcv(struct sock *sk, struct sk_buff *skb)
}
peer_type = hdr->other_pep_type << 8;
- if (unlikely(sk->sk_state != TCP_LISTEN) || sk_acceptq_is_full(sk)) {
- pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
- return -ENOBUFS;
- }
-
/* Parse sub-blocks (options) */
n_sb = hdr->data[4];
while (n_sb > 0) {
@@ -790,7 +786,6 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
struct sock *sknode;
struct pnpipehdr *hdr;
struct sockaddr_pn dst;
- int err = NET_RX_SUCCESS;
u8 pipe_handle;
if (!pskb_may_pull(skb, sizeof(*hdr)))
@@ -814,18 +809,20 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
sock_put(sknode);
if (net_ratelimit())
printk(KERN_WARNING"Phonet unconnected PEP ignored");
- err = NET_RX_DROP;
goto drop;
}
switch (hdr->message_id) {
case PNS_PEP_CONNECT_REQ:
- err = pep_connreq_rcv(sk, skb);
+ if (sk->sk_state == TCP_LISTEN && !sk_acceptq_is_full(sk))
+ pep_connreq_rcv(sk, skb);
+ else
+ pep_reject_conn(sk, skb, PN_PIPE_ERR_PEP_IN_USE);
break;
#ifdef CONFIG_PHONET_PIPECTRLR
case PNS_PEP_CONNECT_RESP:
- err = pep_connresp_rcv(sk, skb);
+ pep_connresp_rcv(sk, skb);
break;
#endif
@@ -842,11 +839,11 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
case PNS_PEP_DISABLE_REQ:
/* invalid handle is not even allowed here! */
default:
- err = NET_RX_DROP;
+ break;
}
drop:
kfree_skb(skb);
- return err;
+ return NET_RX_SUCCESS;
}
#ifndef CONFIG_PHONET_PIPECTRLR
--
1.7.1
^ permalink raw reply related
* [PATCH 8/8] Phonet: kill the ST-Ericsson pipe controller Kconfig
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
This is now a run-time choice so that a single kernel can support both
old and new generation ISI modems.
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
Documentation/networking/phonet.txt | 13 -------------
include/linux/phonet.h | 2 --
net/phonet/Kconfig | 12 ------------
net/phonet/pep.c | 25 -------------------------
4 files changed, 0 insertions(+), 52 deletions(-)
diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index 3d12779..8100358 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -205,19 +205,6 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
socket descriptors that are already connected or being connected.
-Phonet Pipe-controller Implementation
--------------------------------------
-
-Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR
-Kconfig option.
-
-The implementation adds socket options at SOL_PNPIPE level:
-
- PNPIPE_ENABLE accepts one integer value (int). If set to zero, the pipe
- is disabled. If the value is non-zero, the pipe is enabled. If the pipe
- is not (yet) connected, ENOTCONN is error is returned.
-
-
Authors
-------
diff --git a/include/linux/phonet.h b/include/linux/phonet.h
index 32a0965..6fb1384 100644
--- a/include/linux/phonet.h
+++ b/include/linux/phonet.h
@@ -37,8 +37,6 @@
#define PNPIPE_ENCAP 1
#define PNPIPE_IFINDEX 2
#define PNPIPE_HANDLE 3
-#define PNPIPE_ENABLE 4
-/* unused slot */
#define PNADDR_ANY 0
#define PNADDR_BROADCAST 0xFC
diff --git a/net/phonet/Kconfig b/net/phonet/Kconfig
index 0d9b8a2..6ec7d55 100644
--- a/net/phonet/Kconfig
+++ b/net/phonet/Kconfig
@@ -14,15 +14,3 @@ config PHONET
To compile this driver as a module, choose M here: the module
will be called phonet. If unsure, say N.
-
-config PHONET_PIPECTRLR
- bool "Phonet Pipe Controller (EXPERIMENTAL)"
- depends on PHONET && EXPERIMENTAL
- default N
- help
- The Pipe Controller implementation in Phonet stack to support Pipe
- data with Nokia Slim modems like WG2.5 used on ST-Ericsson U8500
- platform.
-
- This option is incompatible with older Nokia modems.
- Say N here unless you really know what you are doing.
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index 671effb..68e635f 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -167,15 +167,6 @@ static int pipe_handler_send_created_ind(struct sock *sk)
data, 4, GFP_ATOMIC);
}
-#ifdef CONFIG_PHONET_PIPECTRLR
-static int pipe_handler_enable_pipe(struct sock *sk, int enable)
-{
- u8 id = enable ? PNS_PEP_ENABLE_REQ : PNS_PEP_DISABLE_REQ;
-
- return pipe_handler_request(sk, id, PAD, NULL, 0);
-}
-#endif
-
static int pep_accept_conn(struct sock *sk, struct sk_buff *skb)
{
static const u8 data[20] = {
@@ -968,16 +959,6 @@ static int pep_setsockopt(struct sock *sk, int level, int optname,
}
goto out_norel;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNPIPE_ENABLE:
- if ((1 << sk->sk_state) & ~(TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
- err = -ENOTCONN;
- break;
- }
- err = pipe_handler_enable_pipe(sk, val);
- break;
-#endif
-
default:
err = -ENOPROTOOPT;
}
@@ -1013,12 +994,6 @@ static int pep_getsockopt(struct sock *sk, int level, int optname,
return -EINVAL;
break;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNPIPE_ENABLE:
- val = sk->sk_state == TCP_ESTABLISHED;
- break;
-#endif
-
default:
return -ENOPROTOOPT;
}
--
1.7.1
^ permalink raw reply related
* [PATCH 7/8] Phonet: support active connection without pipe controller on modem
From: Rémi Denis-Courmont @ 2011-03-08 13:57 UTC (permalink / raw)
To: netdev
In-Reply-To: <201103081556.18877.remi.denis-courmont@nokia.com>
This provides support for newer ISI modems with no need for the
earlier experimental compile-time alternative choice. With this,
we can now use the same kernel and userspace with both types of
modems.
This also avoids confusing two different and incompatible state
machines, actively connected vs accepted sockets, and adds
connection response error handling (processing "SYN/RST" of sorts).
Signed-off-by: Rémi Denis-Courmont <remi.denis-courmont@nokia.com>
---
Documentation/networking/phonet.txt | 53 +++++------
net/phonet/pep.c | 172 ++++++++++++++++++++--------------
net/phonet/socket.c | 102 ++++++++-------------
3 files changed, 165 insertions(+), 162 deletions(-)
diff --git a/Documentation/networking/phonet.txt b/Documentation/networking/phonet.txt
index cacac96..3d12779 100644
--- a/Documentation/networking/phonet.txt
+++ b/Documentation/networking/phonet.txt
@@ -154,9 +154,28 @@ connections, one per accept()'d socket.
write(cfd, msg, msglen);
}
-Connections are established between two endpoints by a "third party"
-application. This means that both endpoints are passive; so connect()
-is not possible.
+Connections are traditionally established between two endpoints by a
+"third party" application. This means that both endpoints are passive.
+
+
+As of Linux kernel version 2.6.39, it is also possible to connect
+two endpoints directly, using connect() on the active side. This is
+intended to support the newer Nokia Wireless Modem API, as found in
+e.g. the Nokia Slim Modem in the ST-Ericsson U8500 platform:
+
+ struct sockaddr_spn spn;
+ int fd;
+
+ fd = socket(PF_PHONET, SOCK_SEQPACKET, PN_PROTO_PIPE);
+ memset(&spn, 0, sizeof(spn));
+ spn.spn_family = AF_PHONET;
+ spn.spn_obj = ...;
+ spn.spn_dev = ...;
+ spn.spn_resource = 0xD9;
+ connect(fd, (struct sockaddr *)&spn, sizeof(spn));
+ /* normal I/O here ... */
+ close(fd);
+
WARNING:
When polling a connected pipe socket for writability, there is an
@@ -189,17 +208,8 @@ The pipe protocol provides two socket options at the SOL_PNPIPE level:
Phonet Pipe-controller Implementation
-------------------------------------
-Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR Kconfig
-option. It is useful when communicating with those Nokia Modems which do not
-implement Pipe controller in them e.g. Nokia Slim Modem used in ST-Ericsson
-U8500 platform.
-
-The implementation is based on the Data Connection Establishment Sequence
-depicted in 'Nokia Wireless Modem API - Wireless_modem_user_guide.pdf'
-document.
-
-It allows a phonet sequenced socket (host-pep) to initiate a Pipe connection
-between itself and a remote pipe-end point (e.g. modem).
+Phonet Pipe-controller is enabled by selecting the CONFIG_PHONET_PIPECTRLR
+Kconfig option.
The implementation adds socket options at SOL_PNPIPE level:
@@ -207,21 +217,6 @@ The implementation adds socket options at SOL_PNPIPE level:
is disabled. If the value is non-zero, the pipe is enabled. If the pipe
is not (yet) connected, ENOTCONN is error is returned.
-The implementation also adds socket 'connect'. On calling the 'connect', pipe
-will be created between the source socket and the destination, and the pipe
-state will be set to PIPE_DISABLED.
-
-After a pipe has been created and enabled successfully, the Pipe data can be
-exchanged between the host-pep and remote-pep (modem).
-
-User-space would typically follow below sequence with Pipe controller:-
--socket
--bind
--setsockopt for PNPIPE_PIPE_HANDLE
--connect
--setsockopt for PNPIPE_ENCAP_IP
--setsockopt for PNPIPE_ENABLE
-
Authors
-------
diff --git a/net/phonet/pep.c b/net/phonet/pep.c
index abfb795..671effb 100644
--- a/net/phonet/pep.c
+++ b/net/phonet/pep.c
@@ -136,7 +136,6 @@ static int pep_indicate(struct sock *sk, u8 id, u8 code,
#define PAD 0x00
-#ifdef CONFIG_PHONET_PIPECTRLR
static int pipe_handler_request(struct sock *sk, u8 id, u8 code,
const void *data, int len)
{
@@ -168,11 +167,7 @@ static int pipe_handler_send_created_ind(struct sock *sk)
data, 4, GFP_ATOMIC);
}
-static int pipe_handler_send_ind(struct sock *sk, u8 id)
-{
- return pep_indicate(sk, id, PAD, NULL, 0, GFP_ATOMIC);
-}
-
+#ifdef CONFIG_PHONET_PIPECTRLR
static int pipe_handler_enable_pipe(struct sock *sk, int enable)
{
u8 id = enable ? PNS_PEP_ENABLE_REQ : PNS_PEP_DISABLE_REQ;
@@ -376,32 +371,11 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
sk->sk_state_change(sk);
break;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNS_PEP_DISCONNECT_RESP:
- sk->sk_state = TCP_CLOSE;
- break;
-#endif
-
case PNS_PEP_ENABLE_REQ:
/* Wait for PNS_PIPE_(ENABLED|REDIRECTED)_IND */
pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
break;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNS_PEP_ENABLE_RESP:
- pipe_handler_send_ind(sk, PNS_PIPE_ENABLED_IND);
-
- if (!pn_flow_safe(pn->tx_fc)) {
- atomic_set(&pn->tx_credits, 1);
- sk->sk_write_space(sk);
- }
- if (sk->sk_state == TCP_ESTABLISHED)
- break; /* Nothing to do */
- sk->sk_state = TCP_ESTABLISHED;
- pipe_grant_credits(sk, GFP_ATOMIC);
- break;
-#endif
-
case PNS_PEP_RESET_REQ:
switch (hdr->state_after_reset) {
case PN_PIPE_DISABLE:
@@ -420,15 +394,6 @@ static int pipe_do_rcv(struct sock *sk, struct sk_buff *skb)
pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
break;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNS_PEP_DISABLE_RESP:
- atomic_set(&pn->tx_credits, 0);
- pipe_handler_send_ind(sk, PNS_PIPE_DISABLED_IND);
- sk->sk_state = TCP_SYN_RECV;
- pn->rx_credits = 0;
- break;
-#endif
-
case PNS_PEP_CTRL_REQ:
if (skb_queue_len(&pn->ctrlreq_queue) >= PNPIPE_CTRLREQ_MAX) {
atomic_inc(&sk->sk_drops);
@@ -521,7 +486,6 @@ static void pipe_destruct(struct sock *sk)
skb_queue_purge(&pn->ctrlreq_queue);
}
-#ifdef CONFIG_PHONET_PIPECTRLR
static u8 pipe_negotiate_fc(const u8 *fcs, unsigned n)
{
unsigned i;
@@ -546,6 +510,8 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
return -EINVAL;
hdr = pnp_hdr(skb);
+ if (hdr->error_code != PN_PIPE_NO_ERROR)
+ return -ECONNREFUSED;
/* Parse sub-blocks */
n_sb = hdr->data[4];
@@ -573,14 +539,74 @@ static int pep_connresp_rcv(struct sock *sk, struct sk_buff *skb)
n_sb--;
}
- sk->sk_state = TCP_SYN_RECV;
- sk->sk_backlog_rcv = pipe_do_rcv;
- pn->rx_credits = 0;
- sk->sk_state_change(sk);
-
return pipe_handler_send_created_ind(sk);
}
-#endif
+
+/* Queue an skb to an actively connected sock.
+ * Socket lock must be held. */
+static int pipe_handler_do_rcv(struct sock *sk, struct sk_buff *skb)
+{
+ struct pep_sock *pn = pep_sk(sk);
+ struct pnpipehdr *hdr = pnp_hdr(skb);
+ int err = NET_RX_SUCCESS;
+
+ switch (hdr->message_id) {
+ case PNS_PIPE_ALIGNED_DATA:
+ __skb_pull(skb, 1);
+ /* fall through */
+ case PNS_PIPE_DATA:
+ __skb_pull(skb, 3); /* Pipe data header */
+ if (!pn_flow_safe(pn->rx_fc)) {
+ err = sock_queue_rcv_skb(sk, skb);
+ if (!err)
+ return NET_RX_SUCCESS;
+ err = NET_RX_DROP;
+ break;
+ }
+
+ if (pn->rx_credits == 0) {
+ atomic_inc(&sk->sk_drops);
+ err = NET_RX_DROP;
+ break;
+ }
+ pn->rx_credits--;
+ skb->dev = NULL;
+ skb_set_owner_r(skb, sk);
+ err = skb->len;
+ skb_queue_tail(&sk->sk_receive_queue, skb);
+ if (!sock_flag(sk, SOCK_DEAD))
+ sk->sk_data_ready(sk, err);
+ return NET_RX_SUCCESS;
+
+ case PNS_PEP_CONNECT_RESP:
+ if (sk->sk_state != TCP_SYN_SENT)
+ break;
+ if (!sock_flag(sk, SOCK_DEAD))
+ sk->sk_state_change(sk);
+ if (pep_connresp_rcv(sk, skb)) {
+ sk->sk_state = TCP_CLOSE_WAIT;
+ break;
+ }
+
+ sk->sk_state = TCP_ESTABLISHED;
+ if (!pn_flow_safe(pn->tx_fc)) {
+ atomic_set(&pn->tx_credits, 1);
+ sk->sk_write_space(sk);
+ }
+ pipe_grant_credits(sk, GFP_ATOMIC);
+ break;
+
+ case PNS_PEP_DISCONNECT_RESP:
+ /* sock should already be dead, nothing to do */
+ break;
+
+ case PNS_PEP_STATUS_IND:
+ pipe_rcv_status(sk, skb);
+ break;
+ }
+ kfree_skb(skb);
+ return err;
+}
/* Listening sock must be locked */
static struct sock *pep_find_pipe(const struct hlist_head *hlist,
@@ -649,12 +675,6 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
sk->sk_data_ready(sk, 0);
return NET_RX_SUCCESS;
-#ifdef CONFIG_PHONET_PIPECTRLR
- case PNS_PEP_CONNECT_RESP:
- pep_connresp_rcv(sk, skb);
- break;
-#endif
-
case PNS_PEP_DISCONNECT_REQ:
pep_reply(sk, skb, PN_PIPE_NO_ERROR, NULL, 0, GFP_ATOMIC);
break;
@@ -667,15 +687,19 @@ static int pep_do_rcv(struct sock *sk, struct sk_buff *skb)
case PNS_PEP_ENABLE_REQ:
case PNS_PEP_DISABLE_REQ:
/* invalid handle is not even allowed here! */
- default:
break;
+
+ default:
+ if ((1 << sk->sk_state)
+ & ~(TCPF_CLOSE|TCPF_LISTEN|TCPF_CLOSE_WAIT))
+ /* actively connected socket */
+ return pipe_handler_do_rcv(sk, skb);
}
drop:
kfree_skb(skb);
return NET_RX_SUCCESS;
}
-#ifndef CONFIG_PHONET_PIPECTRLR
static int pipe_do_remove(struct sock *sk)
{
struct pep_sock *pn = pep_sk(sk);
@@ -693,7 +717,6 @@ static int pipe_do_remove(struct sock *sk)
ph->data[0] = PAD;
return pn_skb_send(sk, skb, NULL);
}
-#endif
/* associated socket ceases to exist */
static void pep_sock_close(struct sock *sk, long timeout)
@@ -706,13 +729,12 @@ static void pep_sock_close(struct sock *sk, long timeout)
lock_sock(sk);
if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED)) {
-#ifndef CONFIG_PHONET_PIPECTRLR
- /* Forcefully remove dangling Phonet pipe */
- pipe_do_remove(sk);
-#else
- /* send pep disconnect request */
- pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD, NULL, 0);
-#endif
+ if (sk->sk_backlog_rcv == pipe_do_rcv)
+ /* Forcefully remove dangling Phonet pipe */
+ pipe_do_remove(sk);
+ else
+ pipe_handler_request(sk, PNS_PEP_DISCONNECT_REQ, PAD,
+ NULL, 0);
}
sk->sk_state = TCP_CLOSE;
@@ -844,20 +866,22 @@ drop:
return newsk;
}
-#ifdef CONFIG_PHONET_PIPECTRLR
static int pep_sock_connect(struct sock *sk, struct sockaddr *addr, int len)
{
struct pep_sock *pn = pep_sk(sk);
- const struct sockaddr_pn *spn = (struct sockaddr_pn *)addr;
+ int err;
u8 data[4] = { 0 /* sub-blocks */, PAD, PAD, PAD };
- pn->pn_sk.dobject = pn_sockaddr_get_object(spn);
- pn->pn_sk.resource = pn_sockaddr_get_resource(spn);
pn->pipe_handle = 1; /* anything but INVALID_HANDLE */
- return pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
- PN_PIPE_DISABLE, data, 4);
+ err = pipe_handler_request(sk, PNS_PEP_CONNECT_REQ,
+ PN_PIPE_ENABLE, data, 4);
+ if (err) {
+ pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
+ return err;
+ }
+ sk->sk_state = TCP_SYN_SENT;
+ return 0;
}
-#endif
static int pep_ioctl(struct sock *sk, int cmd, unsigned long arg)
{
@@ -890,8 +914,16 @@ static int pep_init(struct sock *sk)
sk->sk_destruct = pipe_destruct;
INIT_HLIST_HEAD(&pn->hlist);
+ pn->listener = NULL;
skb_queue_head_init(&pn->ctrlreq_queue);
+ atomic_set(&pn->tx_credits, 0);
+ pn->ifindex = 0;
+ pn->peer_type = 0;
pn->pipe_handle = PN_PIPE_INVALID_HANDLE;
+ pn->rx_credits = 0;
+ pn->rx_fc = pn->tx_fc = PN_LEGACY_FLOW_CONTROL;
+ pn->init_enable = 1;
+ pn->aligned = 0;
return 0;
}
@@ -1219,9 +1251,9 @@ static void pep_sock_unhash(struct sock *sk)
lock_sock(sk);
-#ifndef CONFIG_PHONET_PIPECTRLR
- if ((1 << sk->sk_state) & ~(TCPF_CLOSE|TCPF_LISTEN)) {
+ if (pn->listener != NULL) {
skparent = pn->listener;
+ pn->listener = NULL;
release_sock(sk);
pn = pep_sk(skparent);
@@ -1229,7 +1261,7 @@ static void pep_sock_unhash(struct sock *sk)
sk_del_node_init(sk);
sk = skparent;
}
-#endif
+
/* Unhash a listening sock only when it is closed
* and all of its active connected pipes are closed. */
if (hlist_empty(&pn->hlist))
@@ -1243,9 +1275,7 @@ static void pep_sock_unhash(struct sock *sk)
static struct proto pep_proto = {
.close = pep_sock_close,
.accept = pep_sock_accept,
-#ifdef CONFIG_PHONET_PIPECTRLR
.connect = pep_sock_connect,
-#endif
.ioctl = pep_ioctl,
.init = pep_init,
.setsockopt = pep_setsockopt,
diff --git a/net/phonet/socket.c b/net/phonet/socket.c
index 1eccfc3..b1adafa 100644
--- a/net/phonet/socket.c
+++ b/net/phonet/socket.c
@@ -225,15 +225,18 @@ static int pn_socket_autobind(struct socket *sock)
return 0; /* socket was already bound */
}
-#ifdef CONFIG_PHONET_PIPECTRLR
static int pn_socket_connect(struct socket *sock, struct sockaddr *addr,
int len, int flags)
{
struct sock *sk = sock->sk;
+ struct pn_sock *pn = pn_sk(sk);
struct sockaddr_pn *spn = (struct sockaddr_pn *)addr;
- long timeo;
+ struct task_struct *tsk = current;
+ long timeo = sock_rcvtimeo(sk, flags & O_NONBLOCK);
int err;
+ if (pn_socket_autobind(sock))
+ return -ENOBUFS;
if (len < sizeof(struct sockaddr_pn))
return -EINVAL;
if (spn->spn_family != AF_PHONET)
@@ -243,82 +246,61 @@ static int pn_socket_connect(struct socket *sock, struct sockaddr *addr,
switch (sock->state) {
case SS_UNCONNECTED:
- sk->sk_state = TCP_CLOSE;
- break;
- case SS_CONNECTING:
- switch (sk->sk_state) {
- case TCP_SYN_RECV:
- sock->state = SS_CONNECTED;
- err = -EISCONN;
- goto out;
- case TCP_CLOSE:
- err = -EALREADY;
- if (flags & O_NONBLOCK)
- goto out;
- goto wait_connect;
- }
- break;
- case SS_CONNECTED:
- switch (sk->sk_state) {
- case TCP_SYN_RECV:
+ if (sk->sk_state != TCP_CLOSE) {
err = -EISCONN;
goto out;
- case TCP_CLOSE:
- sock->state = SS_UNCONNECTED;
- break;
}
break;
- case SS_DISCONNECTING:
- case SS_FREE:
- break;
+ case SS_CONNECTING:
+ err = -EALREADY;
+ goto out;
+ default:
+ err = -EISCONN;
+ goto out;
}
- sk->sk_state = TCP_CLOSE;
- sk_stream_kill_queues(sk);
+ pn->dobject = pn_sockaddr_get_object(spn);
+ pn->resource = pn_sockaddr_get_resource(spn);
sock->state = SS_CONNECTING;
+
err = sk->sk_prot->connect(sk, addr, len);
- if (err < 0) {
+ if (err) {
sock->state = SS_UNCONNECTED;
- sk->sk_state = TCP_CLOSE;
+ pn->dobject = 0;
goto out;
}
- err = -EINPROGRESS;
-wait_connect:
- if (sk->sk_state != TCP_SYN_RECV && (flags & O_NONBLOCK))
- goto out;
-
- timeo = sock_sndtimeo(sk, flags & O_NONBLOCK);
- release_sock(sk);
+ while (sk->sk_state == TCP_SYN_SENT) {
+ DEFINE_WAIT(wait);
- err = -ERESTARTSYS;
- timeo = wait_event_interruptible_timeout(*sk_sleep(sk),
- sk->sk_state != TCP_CLOSE,
- timeo);
-
- lock_sock(sk);
- if (timeo < 0)
- goto out; /* -ERESTARTSYS */
-
- err = -ETIMEDOUT;
- if (timeo == 0 && sk->sk_state != TCP_SYN_RECV)
- goto out;
+ if (!timeo) {
+ err = -EINPROGRESS;
+ goto out;
+ }
+ if (signal_pending(tsk)) {
+ err = sock_intr_errno(timeo);
+ goto out;
+ }
- if (sk->sk_state != TCP_SYN_RECV) {
- sock->state = SS_UNCONNECTED;
- err = sock_error(sk);
- if (!err)
- err = -ECONNREFUSED;
- goto out;
+ prepare_to_wait_exclusive(sk_sleep(sk), &wait,
+ TASK_INTERRUPTIBLE);
+ release_sock(sk);
+ timeo = schedule_timeout(timeo);
+ lock_sock(sk);
+ finish_wait(sk_sleep(sk), &wait);
}
- sock->state = SS_CONNECTED;
- err = 0;
+ if ((1 << sk->sk_state) & (TCPF_SYN_RECV|TCPF_ESTABLISHED))
+ err = 0;
+ else if (sk->sk_state == TCP_CLOSE_WAIT)
+ err = -ECONNRESET;
+ else
+ err = -ECONNREFUSED;
+ sock->state = err ? SS_UNCONNECTED : SS_CONNECTED;
out:
release_sock(sk);
return err;
}
-#endif
static int pn_socket_accept(struct socket *sock, struct socket *newsock,
int flags)
@@ -486,11 +468,7 @@ const struct proto_ops phonet_stream_ops = {
.owner = THIS_MODULE,
.release = pn_socket_release,
.bind = pn_socket_bind,
-#ifdef CONFIG_PHONET_PIPECTRLR
.connect = pn_socket_connect,
-#else
- .connect = sock_no_connect,
-#endif
.socketpair = sock_no_socketpair,
.accept = pn_socket_accept,
.getname = pn_socket_getname,
--
1.7.1
^ permalink raw reply related
* Re: [Lxc-users] Bad checksums and lost packets with macvlan on dummy
From: Patrick McHardy @ 2011-03-08 14:41 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: Eric Dumazet, Andrian Nord, lxc-users, Linux Netdev List
In-Reply-To: <4D6E8D80.3090907@free.fr>
Am 02.03.2011 19:33, schrieb Daniel Lezcano:
> On 03/02/2011 07:03 PM, Patrick McHardy wrote:
>> Am 02.03.2011 17:03, schrieb Daniel Lezcano:
>>> On 03/02/2011 12:03 PM, Patrick McHardy wrote:
>>>> Am 01.03.2011 21:04, schrieb Daniel Lezcano:
>>>>> On 03/01/2011 05:51 PM, Patrick McHardy wrote:
>>>>>>> Patrick, do you have any suggestions to fix this ?
>>>>>> Since the frames are only looped back locally, I suppose the easiest
>>>>>> fix would be to mark them with CHECKSUM_UNNECESSARY. Alternatively
>>>>>> we need to complete the checksum manually, similar to what
>>>>>> dev_hard_start_xmit() does.
>>>>> That sounds very simple to fix, maybe too much simple :)
>>>>>
>>>>> I did the following change:
>>>>>
>>>>> --- linux-next.orig/drivers/net/macvlan.c
>>>>> +++ linux-next/drivers/net/macvlan.c
>>>>> @@ -222,6 +222,7 @@ static int macvlan_queue_xmit(struct sk_
>>>>>
>>>>> if (vlan->mode == MACVLAN_MODE_BRIDGE) {
>>>>> const struct ethhdr *eth = (void *)skb->data;
>>>>> + skb->ip_summed = CHECKSUM_UNNECESSARY;
>>>>>
>>>>> /* send to other bridge ports directly */
>>>>> if (is_multicast_ether_addr(eth->h_dest)) {
>>>>>
>>>>>
>>>>> and that fixed the problem. Do you think it is acceptable ?
>>>> The only problem I see is if the packets are bridged to a
>>>> different networking device (or redirected using the mirred
>>>> action), in this case the checksum will not be completed.
>>>> This would be a very strange setup though and probably wouldn't
>>>> be using dummy as lower device, so I'm not sure we have to
>>>> worry about this case.
>>> I am not sure to get it, do you say the patch is correct ?
>> Its correct with a short-coming that doesn't seem to matter.
>>
>>> If my understanding is correct, the packet will be flagged
>>> CHECKSUM_UNNECESSARY only for the macvlan devices, right ?
>> Only for packets bridged between macvlan devices. A setup like
>> the following would cause problems:
>>
>> br0
>> |
>> .----------.
>> | |
>> macvlan0 macvlan1 eth0
>> | |
>> -------.-------
>> dummy0
>>
>> In this case packets sent from macvlan0 will show up on
>> eth0 with incorrect setups. However this setup doesn't
>> seem realistic to me, you would simply use eth0 instead
>> of dummy0.
>
> Ok, I understand. thanks for the clarification.
>
>>> By the way, this problem occurs for any lower device with offloading
>>> capabilities with a macvlan port in bridge mode.
>> True. This doesn't affect outgoing packets since their checksum
>> will be completed in dev_hard_start_xmit(), but it affects
>> packets bridged between macvlans.
>
> One last question. In the case of broadcast packets with maclvan in
> bridge mode.
> We will have the packets going through each macvlan port and also to the
> lower-device, right ?
> For the latter, don't we have a problem if the packet is flagged
> CHECKSUM_UNNECESSARY ?
>
> Shouldn't we restore the ip_summed field before sending through
> dev_queue_xmit ?
Yes, that seems correct in order to have dev_hard_start_xmit() complete
the checksum if necessary.
^ permalink raw reply
* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Patrick McHardy @ 2011-03-08 14:50 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <20110304012956.GA13573@ioremap.net>
Am 04.03.2011 02:29, schrieb Evgeniy Polyakov:
> Hi.
>
> On Thu, Mar 03, 2011 at 11:37:46PM +0100, Lars Ellenberg (lars.ellenberg@linbit.com) wrote:
>> If so, then this change introduces the possibility for normal users to
>> send privileged commands to connector based subsystems, even if they
>> may not be able to bind() to suitable sockets to receive any replies.
>>
>> Am I missing something?
>
> Yup, connector is very async at that place, but I wonder why the hell I
> ever made that decision. I believe we can replace it with pure sync call
> of the registered connector callback, since netlink is synchronous and
> no one has any problem with it.
>
Are you going to do this or do you want me to take care of it?
^ permalink raw reply
* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Injong Rhee @ 2011-03-08 15:26 UTC (permalink / raw)
To: Lucas Nussbaum; +Cc: WANG Cong, netdev
In-Reply-To: <20110308111011.GA27967@xanadu.blop.info>
Thanks for updating CUBIC hystart. You might want to test the
cases with more background traffic and verify whether this
threshold is too conservative.
On 3/8/11 6:10 AM, Lucas Nussbaum wrote:
> CUBIC Hystart uses two heuristics to exit slow start earlier, before
> losses start to occur. Unfortunately, it tends to exit slow start far
> too early, causing poor performance since convergence to the optimal
> cwnd is then very slow. This was reported in
> http://permalink.gmane.org/gmane.linux.network/188169 and
> https://partner-bugzilla.redhat.com/show_bug.cgi?id=616985
>
> I am using an experimental testbed (http://www.grid5000.fr/) with two
> machines connected using Gigabit ethernet to a dedicated 10-Gb backbone.
> RTT between both machines is 11.3ms. Using TCP CUBIC without Hystart,
> cwnd grows to ~2200. With Hystart enabled, CUBIC exits slow start with
> cwnd lower than 100, and often lower than 20, which leads to the poor
> performance that I reported.
>
> After instrumenting TCP CUBIC, I found out that the segment-to-ack RTT
> tends to vary quite a lot even when the network is not congested, due to
> several factors including the fact that TCP sends packet in burst (so
> the packets are queued locally before being sent, increasing their RTT),
> and delayed ACKs on the destination host.
>
> The patch below increases the thresholds used by the two Hystart
> heuristics. First, the length of an ACK train needs to reach 2*minRTT.
> Second, the max RTT of a group of packets also needs to reach 2*minRTT.
> In my setup, this causes Hystart to exit slow start when cwnd is in the
> 1900-2000 range using the ACK train heuristics, and sometimes to exit in
> the 700-900 range using the delay increase heuristic, dramatically
> improving performance.
>
> I could provide access to my testbed if someone wants to do further
> experiments.
>
> Signed-off-by: Lucas Nussbaum<lucas.nussbaum@loria.fr>
^ permalink raw reply
* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Krishna Kumar2 @ 2011-03-08 15:32 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <20110302100600.GB31309@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> wrote on 03/02/2011 03:36:00 PM:
Sorry for the delayed response, I have been sick the last few days.
I am responding to both your posts here.
> > Both virtio-net and vhost need some check to make sure very
> > high values are not passed by userspace. Is this not required?
>
> Whatever we stick in the header is effectively part of
> host/gues interface. Are you sure we'll never want
> more than 16 VQs? This value does not seem that high.
OK, so even constants cannot change? Given that, should I remove all
checks and use kcalloc?
> > OK, so virtio_net_config has num_queue_pairs, and this gets converted to
> > numtxqs in virtnet_info?
>
> Or put num_queue_pairs in virtnet_info too.
For virtnet_info, having numtxqs is easier since all code that loops needs
only 'numtxq'.
> > Also, vhost has some
> > code that processes tx first before rx (e.g. vhost_net_stop/flush),
>
> No idea why did I do it this way. I don't think it matters.
>
> > so this approach seemed helpful.
> > I am OK either way, what do you
> > suggest?
>
> We get less code generated but also less flexibility.
> I am not sure, I'll play around with code, for now
> let's keep it as is.
OK.
> > Yes, it is a waste to have these vectors for tx ints. I initially
> > thought of adding a flag to virtio_device to pass to vp_find_vqs,
> > but it won't work, so a new API is needed. I can work with you on
> > this in the background if you like.
>
> OK. For starters, how about we change find_vqs to get a structure? Then
> we can easily add flags that tell us that some interrupts are rare.
Yes. OK to work on this outside this patch series, I guess?
> > vq's are matched between qemu, virtio-net and vhost. Isn't some check
> > required that userspace has not passed a bad value?
>
>
> For virtio, I'm not too concerned: qemu can already easily
> crash the guest :)
> For vhost yes, but I'm concerned that even with 16 VQs we are
> drinking a lot of resources already. I would be happier
> if we had a file descriptor per VQs pair in some way.
> The the amount of memory userspace can use up is
> limited by the # of file descriptors.
I will start working on this approach this week and see how it goes.
> > OK, so define free_unused_bufs() as:
> >
> > static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
> > *svq,
> > struct virtqueue *rvq)
> > {
> > /* Use svq and rvq with the remaining code unchanged */
> > }
>
> Not sure I understand. I am just suggesting
> adding symmetrical functions like init/cleanup
> alloc/free etc instead of adding stuff in random
> functions that just happens to be called at the right time.
OK, I will clean up this part in the next revision.
> > I was not sure what is the best way - a sysctl parameter? Or should the
> > maximum depend on number of host cpus? But that results in too many
> > threads, e.g. if I have 16 cpus and 16 txqs.
>
> I guess the question is, wouldn't # of threads == # of vqs work best?
> If we process stuff on a single CPU, let's make it pass through
> a single VQ.
> And to do this, we could simply open multiple vhost fds without
> changing vhost at all.
>
> Would this work well?
>
> > > > - enum vhost_net_poll_state tx_poll_state;
> > > > + enum vhost_net_poll_state *tx_poll_state;
> > >
> > > another array?
> >
> > Yes... I am also allocating twice the space than what is required
> > to make it's usage simple.
>
> Where's the allocation? Couldn't find it.
vhost_setup_vqs(net.c) allocates it based on nvqs, though numtxqs is
enough.
Thanks,
- KK
^ permalink raw reply
* Re: [PATCH 2/3] [RFC] Changes for MQ virtio-net
From: Michael S. Tsirkin @ 2011-03-08 15:41 UTC (permalink / raw)
To: Krishna Kumar2
Cc: anthony, arnd, avi, davem, eric.dumazet, horms, kvm, netdev,
rusty
In-Reply-To: <OFCDD0BC16.2EA548B2-ON6525784D.00551582-6525784D.00553911@in.ibm.com>
On Tue, Mar 08, 2011 at 09:02:38PM +0530, Krishna Kumar2 wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote on 03/02/2011 03:36:00 PM:
>
> Sorry for the delayed response, I have been sick the last few days.
> I am responding to both your posts here.
>
> > > Both virtio-net and vhost need some check to make sure very
> > > high values are not passed by userspace. Is this not required?
> >
> > Whatever we stick in the header is effectively part of
> > host/gues interface. Are you sure we'll never want
> > more than 16 VQs? This value does not seem that high.
>
> OK, so even constants cannot change?
Parse error :).
> Given that, should I remove all
> checks and use kcalloc?
I think that it's not a bad idea to avoid crashing if
hardware (in our case, host) misbehaves.
But as long as the code is prepared to handle any # of vqs,
I don't see a point of limiting that arbitrarily,
and if the code to handle hardware errors is complex
it'll have bugs itself.
> > > OK, so virtio_net_config has num_queue_pairs, and this gets converted to
> > > numtxqs in virtnet_info?
> >
> > Or put num_queue_pairs in virtnet_info too.
>
> For virtnet_info, having numtxqs is easier since all code that loops needs
> only 'numtxq'.
It seemed to me that the code used numtxqs for # of rx qs as well
sometimes.
> > > Also, vhost has some
> > > code that processes tx first before rx (e.g. vhost_net_stop/flush),
> >
> > No idea why did I do it this way. I don't think it matters.
> >
> > > so this approach seemed helpful.
> > > I am OK either way, what do you
> > > suggest?
> >
> > We get less code generated but also less flexibility.
> > I am not sure, I'll play around with code, for now
> > let's keep it as is.
>
> OK.
>
> > > Yes, it is a waste to have these vectors for tx ints. I initially
> > > thought of adding a flag to virtio_device to pass to vp_find_vqs,
> > > but it won't work, so a new API is needed. I can work with you on
> > > this in the background if you like.
> >
> > OK. For starters, how about we change find_vqs to get a structure? Then
> > we can easily add flags that tell us that some interrupts are rare.
>
> Yes. OK to work on this outside this patch series, I guess?
We could work on this in parallel. Changing APIs is not a problem,
I'm a bit concerned that this might affect the host/guest ABI as well.
> > > vq's are matched between qemu, virtio-net and vhost. Isn't some check
> > > required that userspace has not passed a bad value?
> >
> >
> > For virtio, I'm not too concerned: qemu can already easily
> > crash the guest :)
> > For vhost yes, but I'm concerned that even with 16 VQs we are
> > drinking a lot of resources already. I would be happier
> > if we had a file descriptor per VQs pair in some way.
> > The the amount of memory userspace can use up is
> > limited by the # of file descriptors.
>
> I will start working on this approach this week and see how it goes.
Also, could you post your current version of the qemu code pls?
It's useful for testing and to see the whole picture.
> > > OK, so define free_unused_bufs() as:
> > >
> > > static void free_unused_bufs(struct virtnet_info *vi, struct virtqueue
> > > *svq,
> > > struct virtqueue *rvq)
> > > {
> > > /* Use svq and rvq with the remaining code unchanged */
> > > }
> >
> > Not sure I understand. I am just suggesting
> > adding symmetrical functions like init/cleanup
> > alloc/free etc instead of adding stuff in random
> > functions that just happens to be called at the right time.
>
> OK, I will clean up this part in the next revision.
>
> > > I was not sure what is the best way - a sysctl parameter? Or should the
> > > maximum depend on number of host cpus? But that results in too many
> > > threads, e.g. if I have 16 cpus and 16 txqs.
> >
> > I guess the question is, wouldn't # of threads == # of vqs work best?
> > If we process stuff on a single CPU, let's make it pass through
> > a single VQ.
> > And to do this, we could simply open multiple vhost fds without
> > changing vhost at all.
> >
> > Would this work well?
> >
> > > > > - enum vhost_net_poll_state tx_poll_state;
> > > > > + enum vhost_net_poll_state *tx_poll_state;
> > > >
> > > > another array?
> > >
> > > Yes... I am also allocating twice the space than what is required
> > > to make it's usage simple.
> >
> > Where's the allocation? Couldn't find it.
>
> vhost_setup_vqs(net.c) allocates it based on nvqs, though numtxqs is
> enough.
>
> Thanks,
>
> - KK
^ permalink raw reply
* Re: [PATCH] vhost: Cleanup vhost.c and net.c
From: Michael S. Tsirkin @ 2011-03-08 17:23 UTC (permalink / raw)
To: Krishna Kumar; +Cc: netdev
In-Reply-To: <20110301113637.4130.38439.sendpatchset@krkumar2.in.ibm.com>
On Tue, Mar 01, 2011 at 05:06:37PM +0530, Krishna Kumar wrote:
> Minor cleanup of vhost.c and net.c to match coding style.
>
> Signed-off-by: Krishna Kumar <krkumar2@in.ibm.com>
Applied, thanks!
> ---
> drivers/vhost/net.c | 19 +++++++++-----
> drivers/vhost/vhost.c | 53 +++++++++++++++++++++++++++-------------
> 2 files changed, 49 insertions(+), 23 deletions(-)
>
> diff -ruNp org/drivers/vhost/net.c new/drivers/vhost/net.c
> --- org/drivers/vhost/net.c 2011-03-01 13:33:25.000000000 +0530
> +++ new/drivers/vhost/net.c 2011-03-01 13:36:44.000000000 +0530
> @@ -60,6 +60,7 @@ static int move_iovec_hdr(struct iovec *
> {
> int seg = 0;
> size_t size;
> +
> while (len && seg < iov_count) {
> size = min(from->iov_len, len);
> to->iov_base = from->iov_base;
> @@ -79,6 +80,7 @@ static void copy_iovec_hdr(const struct
> {
> int seg = 0;
> size_t size;
> +
> while (len && seg < iovcount) {
> size = min(from->iov_len, len);
> to->iov_base = from->iov_base;
> @@ -296,17 +298,16 @@ static void handle_rx_big(struct vhost_n
> .msg_iov = vq->iov,
> .msg_flags = MSG_DONTWAIT,
> };
> -
> struct virtio_net_hdr hdr = {
> .flags = 0,
> .gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> -
> size_t len, total_len = 0;
> int err;
> size_t hdr_size;
> /* TODO: check that we are running from vhost_worker? */
> struct socket *sock = rcu_dereference_check(vq->private_data, 1);
> +
> if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> return;
>
> @@ -405,18 +406,17 @@ static void handle_rx_mergeable(struct v
> .msg_iov = vq->iov,
> .msg_flags = MSG_DONTWAIT,
> };
> -
> struct virtio_net_hdr_mrg_rxbuf hdr = {
> .hdr.flags = 0,
> .hdr.gso_type = VIRTIO_NET_HDR_GSO_NONE
> };
> -
> size_t total_len = 0;
> int err, headcount;
> size_t vhost_hlen, sock_hlen;
> size_t vhost_len, sock_len;
> /* TODO: check that we are running from vhost_worker? */
> struct socket *sock = rcu_dereference_check(vq->private_data, 1);
> +
> if (!sock || skb_queue_empty(&sock->sk->sk_receive_queue))
> return;
>
> @@ -654,6 +654,7 @@ static struct socket *get_raw_socket(int
> } uaddr;
> int uaddr_len = sizeof uaddr, r;
> struct socket *sock = sockfd_lookup(fd, &r);
> +
> if (!sock)
> return ERR_PTR(-ENOTSOCK);
>
> @@ -682,6 +683,7 @@ static struct socket *get_tap_socket(int
> {
> struct file *file = fget(fd);
> struct socket *sock;
> +
> if (!file)
> return ERR_PTR(-EBADF);
> sock = tun_get_socket(file);
> @@ -696,6 +698,7 @@ static struct socket *get_tap_socket(int
> static struct socket *get_socket(int fd)
> {
> struct socket *sock;
> +
> /* special case to disable backend */
> if (fd == -1)
> return NULL;
> @@ -741,9 +744,9 @@ static long vhost_net_set_backend(struct
> oldsock = rcu_dereference_protected(vq->private_data,
> lockdep_is_held(&vq->mutex));
> if (sock != oldsock) {
> - vhost_net_disable_vq(n, vq);
> - rcu_assign_pointer(vq->private_data, sock);
> - vhost_net_enable_vq(n, vq);
> + vhost_net_disable_vq(n, vq);
> + rcu_assign_pointer(vq->private_data, sock);
> + vhost_net_enable_vq(n, vq);
> }
>
> mutex_unlock(&vq->mutex);
> @@ -768,6 +771,7 @@ static long vhost_net_reset_owner(struct
> struct socket *tx_sock = NULL;
> struct socket *rx_sock = NULL;
> long err;
> +
> mutex_lock(&n->dev.mutex);
> err = vhost_dev_check_owner(&n->dev);
> if (err)
> @@ -829,6 +833,7 @@ static long vhost_net_ioctl(struct file
> struct vhost_vring_file backend;
> u64 features;
> int r;
> +
> switch (ioctl) {
> case VHOST_NET_SET_BACKEND:
> if (copy_from_user(&backend, argp, sizeof backend))
> diff -ruNp org/drivers/vhost/vhost.c new/drivers/vhost/vhost.c
> --- org/drivers/vhost/vhost.c 2011-01-19 20:01:29.000000000 +0530
> +++ new/drivers/vhost/vhost.c 2011-03-01 13:36:58.000000000 +0530
> @@ -41,8 +41,8 @@ static void vhost_poll_func(struct file
> poll_table *pt)
> {
> struct vhost_poll *poll;
> - poll = container_of(pt, struct vhost_poll, table);
>
> + poll = container_of(pt, struct vhost_poll, table);
> poll->wqh = wqh;
> add_wait_queue(wqh, &poll->wait);
> }
> @@ -85,6 +85,7 @@ void vhost_poll_init(struct vhost_poll *
> void vhost_poll_start(struct vhost_poll *poll, struct file *file)
> {
> unsigned long mask;
> +
> mask = file->f_op->poll(file, &poll->table);
> if (mask)
> vhost_poll_wakeup(&poll->wait, 0, 0, (void *)mask);
> @@ -101,6 +102,7 @@ static bool vhost_work_seq_done(struct v
> unsigned seq)
> {
> int left;
> +
> spin_lock_irq(&dev->work_lock);
> left = seq - work->done_seq;
> spin_unlock_irq(&dev->work_lock);
> @@ -222,6 +224,7 @@ static int vhost_worker(void *data)
> static long vhost_dev_alloc_iovecs(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> dev->vqs[i].indirect = kmalloc(sizeof *dev->vqs[i].indirect *
> UIO_MAXIOV, GFP_KERNEL);
> @@ -235,6 +238,7 @@ static long vhost_dev_alloc_iovecs(struc
> goto err_nomem;
> }
> return 0;
> +
> err_nomem:
> for (; i >= 0; --i) {
> kfree(dev->vqs[i].indirect);
> @@ -247,6 +251,7 @@ err_nomem:
> static void vhost_dev_free_iovecs(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> kfree(dev->vqs[i].indirect);
> dev->vqs[i].indirect = NULL;
> @@ -296,26 +301,28 @@ long vhost_dev_check_owner(struct vhost_
> }
>
> struct vhost_attach_cgroups_struct {
> - struct vhost_work work;
> - struct task_struct *owner;
> - int ret;
> + struct vhost_work work;
> + struct task_struct *owner;
> + int ret;
> };
>
> static void vhost_attach_cgroups_work(struct vhost_work *work)
> {
> - struct vhost_attach_cgroups_struct *s;
> - s = container_of(work, struct vhost_attach_cgroups_struct, work);
> - s->ret = cgroup_attach_task_all(s->owner, current);
> + struct vhost_attach_cgroups_struct *s;
> +
> + s = container_of(work, struct vhost_attach_cgroups_struct, work);
> + s->ret = cgroup_attach_task_all(s->owner, current);
> }
>
> static int vhost_attach_cgroups(struct vhost_dev *dev)
> {
> - struct vhost_attach_cgroups_struct attach;
> - attach.owner = current;
> - vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> - vhost_work_queue(dev, &attach.work);
> - vhost_work_flush(dev, &attach.work);
> - return attach.ret;
> + struct vhost_attach_cgroups_struct attach;
> +
> + attach.owner = current;
> + vhost_work_init(&attach.work, vhost_attach_cgroups_work);
> + vhost_work_queue(dev, &attach.work);
> + vhost_work_flush(dev, &attach.work);
> + return attach.ret;
> }
>
> /* Caller should have device mutex */
> @@ -323,11 +330,13 @@ static long vhost_dev_set_owner(struct v
> {
> struct task_struct *worker;
> int err;
> +
> /* Is there an owner already? */
> if (dev->mm) {
> err = -EBUSY;
> goto err_mm;
> }
> +
> /* No owner, become one */
> dev->mm = get_task_mm(current);
> worker = kthread_create(vhost_worker, dev, "vhost-%d", current->pid);
> @@ -380,6 +389,7 @@ long vhost_dev_reset_owner(struct vhost_
> void vhost_dev_cleanup(struct vhost_dev *dev)
> {
> int i;
> +
> for (i = 0; i < dev->nvqs; ++i) {
> if (dev->vqs[i].kick && dev->vqs[i].handle_kick) {
> vhost_poll_stop(&dev->vqs[i].poll);
> @@ -421,6 +431,7 @@ void vhost_dev_cleanup(struct vhost_dev
> static int log_access_ok(void __user *log_base, u64 addr, unsigned long sz)
> {
> u64 a = addr / VHOST_PAGE_SIZE / 8;
> +
> /* Make sure 64 bit math will not overflow. */
> if (a > ULONG_MAX - (unsigned long)log_base ||
> a + (unsigned long)log_base > ULONG_MAX)
> @@ -461,6 +472,7 @@ static int memory_access_ok(struct vhost
> int log_all)
> {
> int i;
> +
> for (i = 0; i < d->nvqs; ++i) {
> int ok;
> mutex_lock(&d->vqs[i].mutex);
> @@ -527,6 +539,7 @@ static long vhost_set_memory(struct vhos
> {
> struct vhost_memory mem, *newmem, *oldmem;
> unsigned long size = offsetof(struct vhost_memory, regions);
> +
> if (copy_from_user(&mem, m, size))
> return -EFAULT;
> if (mem.padding)
> @@ -544,7 +557,8 @@ static long vhost_set_memory(struct vhos
> return -EFAULT;
> }
>
> - if (!memory_access_ok(d, newmem, vhost_has_feature(d, VHOST_F_LOG_ALL))) {
> + if (!memory_access_ok(d, newmem,
> + vhost_has_feature(d, VHOST_F_LOG_ALL))) {
> kfree(newmem);
> return -EFAULT;
> }
> @@ -560,6 +574,7 @@ static int init_used(struct vhost_virtqu
> struct vring_used __user *used)
> {
> int r = put_user(vq->used_flags, &used->flags);
> +
> if (r)
> return r;
> return get_user(vq->last_used_idx, &used->idx);
> @@ -849,6 +864,7 @@ static const struct vhost_memory_region
> {
> struct vhost_memory_region *reg;
> int i;
> +
> /* linear search is not brilliant, but we really have on the order of 6
> * regions in practice */
> for (i = 0; i < mem->nregions; ++i) {
> @@ -871,6 +887,7 @@ static int set_bit_to_user(int nr, void
> void *base;
> int bit = nr + (log % PAGE_SIZE) * 8;
> int r;
> +
> r = get_user_pages_fast(log, 1, 1, &page);
> if (r < 0)
> return r;
> @@ -888,6 +905,7 @@ static int log_write(void __user *log_ba
> {
> u64 write_page = write_address / VHOST_PAGE_SIZE;
> int r;
> +
> if (!write_length)
> return 0;
> write_length += write_address % VHOST_PAGE_SIZE;
> @@ -1037,8 +1055,8 @@ static int get_indirect(struct vhost_dev
> i, count);
> return -EINVAL;
> }
> - if (unlikely(memcpy_fromiovec((unsigned char *)&desc, vq->indirect,
> - sizeof desc))) {
> + if (unlikely(memcpy_fromiovec((unsigned char *)&desc,
> + vq->indirect, sizeof desc))) {
> vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",
> i, (size_t)indirect->addr + i * sizeof desc);
> return -EINVAL;
> @@ -1317,6 +1335,7 @@ int vhost_add_used_n(struct vhost_virtqu
> void vhost_signal(struct vhost_dev *dev, struct vhost_virtqueue *vq)
> {
> __u16 flags;
> +
> /* Flush out used index updates. This is paired
> * with the barrier that the Guest executes when enabling
> * interrupts. */
> @@ -1361,6 +1380,7 @@ bool vhost_enable_notify(struct vhost_vi
> {
> u16 avail_idx;
> int r;
> +
> if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
> return false;
> vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
> @@ -1387,6 +1407,7 @@ bool vhost_enable_notify(struct vhost_vi
> void vhost_disable_notify(struct vhost_virtqueue *vq)
> {
> int r;
> +
> if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
> return;
> vq->used_flags |= VRING_USED_F_NO_NOTIFY;
^ permalink raw reply
* Re: [PATCH 1/4] slub: automatically reserve bytes at the end of slab
From: Christoph Lameter @ 2011-03-08 18:24 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA84B.9080606@cn.fujitsu.com>
On Tue, 1 Mar 2011, Lai Jiangshan wrote:
> So we add a field "reserved" to struct kmem_cache, when a slab
> is allocated, kmem_cache->reserved bytes are automatically reserved
> at the end of the slab for slab's metadata.
The reserved field should be exported via sysfs. Please add the code to
show it in /sys/kernel/slab/<name>
^ permalink raw reply
* Re: [PATCH] Make CUBIC Hystart more robust to RTT variations
From: Lucas Nussbaum @ 2011-03-08 18:14 UTC (permalink / raw)
To: Sangtae Ha; +Cc: WANG Cong, Injong Rhee, Netdev
In-Reply-To: <AANLkTimdpEKHfVKw+bm6OnymcnUrauU+jGOPeLzy3Q0o@mail.gmail.com>
On 08/03/11 at 11:43 -0500, Sangtae Ha wrote:
> Hi Lucas,
>
> The current packet-train threshold and the delay threshold have been tested
> with the bandwidth ranging from 10M to 400M, the RTT from 10ms to 320ms, and
> the buffer size from 10% BDP to 200% BDP and they were set conservatively to
> make it work over the network with very small buffer sizes. I will recreate
> your setup and check whether the current thresholds are too conservative and
> will come up with the patch.
I'm surprised. It's possible that a seemingly unrelated change broke it,
but it was already broken for me on 2.6.32.
I can provide access to the testbed if you want to run tests on it.
--
| Lucas Nussbaum MCF Université Nancy 2 |
| lucas.nussbaum@loria.fr LORIA / AlGorille |
| http://www.loria.fr/~lnussbau/ +33 3 54 95 86 19 |
^ permalink raw reply
* Re: [PATCH 2/4] slub,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-08 18:25 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA852.3060303@cn.fujitsu.com>
On Tue, 1 Mar 2011, Lai Jiangshan wrote:
> - * RCU free overloads the RCU head over the LRU
> - */
> - struct rcu_head *head = (void *)&page->lru;
> + struct rcu_head *head;
> +
> + if (need_reserve_slab_rcu) {
> + int order = compound_order(page);
> + int offset = (PAGE_SIZE << order) - s->reserved;
> +
> + BUG_ON(s->reserved != sizeof(*head));
VM_BUG_ON is sufficient here I think.
Otherwise
Acked-by: Christoph Lameter <cl@linux.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH 3/4] slab,rcu: don't assume the size of struct rcu_head
From: Christoph Lameter @ 2011-03-08 18:27 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Ingo Molnar, Paul E. McKenney, Pekka Enberg, Eric Dumazet,
David S. Miller, Matt Mackall, linux-mm, linux-kernel, netdev
In-Reply-To: <4D6CA85B.5030107@cn.fujitsu.com>
Acked-by: Christoph Lameter <cl@linux.com>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [Drbd-dev] [PATCH 2/2 v2] netlink: kill eff_cap from struct netlink_skb_parms
From: Evgeniy Polyakov @ 2011-03-08 18:32 UTC (permalink / raw)
To: Patrick McHardy
Cc: Chris Wright, David Miller, linux-fbdev, netdev,
linux-security-module, dm-devel, drbd-dev
In-Reply-To: <4D764247.9080509@trash.net>
Hi Patrick.
On Tue, Mar 08, 2011 at 03:50:47PM +0100, Patrick McHardy (kaber@trash.net) wrote:
> > Yup, connector is very async at that place, but I wonder why the hell I
> > ever made that decision. I believe we can replace it with pure sync call
> > of the registered connector callback, since netlink is synchronous and
> > no one has any problem with it.
>
> Are you going to do this or do you want me to take care of it?
I will return back at the end of the week and will take care of this
problem. I will not mind if you complete it first though :)
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH] ipv4: Cache source address in nexthop entries.
From: David Miller @ 2011-03-08 18:38 UTC (permalink / raw)
To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1103081030090.1529@ja.ssi.bg>
From: Julian Anastasov <ja@ssi.bg>
Date: Tue, 8 Mar 2011 11:57:38 +0200 (EET)
> It means, even if there are addresses on the
> concerned device it does not mean the routes on this device
> are required to use prefsrc from this device. We must
> restrict the scope according to the provided for the
> route: cfg->fc_scope.
Simple to fix, I'll remember the fc_scope value in the nexthop
and use that in the address selection call.
> As the addresses have nothing to do with the link
> state, I don't think it is correct to call fib_update_nh_saddrs
> for DEV events.
fib_update_nh_saddrs() gets called for "DEV" events since those
are what are emitted when addresses are addded and removed.
^ 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