* Re: [PATCH net 2/2] ip_tunnel: Add fallback tunnels to the hash lists
From: Steffen Klassert @ 2013-09-26 8:13 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, netdev
In-Reply-To: <CALnjE+rrm3Vzov4sUjBfEKHwzRb6MfTRhkA_B96yuNGpgFgiCg@mail.gmail.com>
On Wed, Sep 25, 2013 at 09:03:11AM -0700, Pravin Shelar wrote:
> On Tue, Sep 24, 2013 at 10:55 PM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > Currently we can not update the tunnel parameters of
> > the fallback tunnels because we don't find them in the
> > hash lists. Fix this by adding them on initialization.
> >
> > Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
> > ---
> > net/ipv4/ip_tunnel.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
> > index b8ce640..f2348f2 100644
> > --- a/net/ipv4/ip_tunnel.c
> > +++ b/net/ipv4/ip_tunnel.c
> > @@ -853,8 +853,10 @@ int ip_tunnel_init_net(struct net *net, int ip_tnl_net_id,
> > /* FB netdevice is special: we have one, and only one per netns.
> > * Allowing to move it to another netns is clearly unsafe.
> > */
> > - if (!IS_ERR(itn->fb_tunnel_dev))
> > + if (!IS_ERR(itn->fb_tunnel_dev)) {
> > itn->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
> > + ip_tunnel_add(itn, netdev_priv(itn->fb_tunnel_dev));
> > + }
> > rtnl_unlock();
> >
> fallback tunnel s not required to be in hash table, Its is returned if
> none of hashed tunnels are matched, ref ip_tunnel_lookup().
> Can you post command to reproduce this issue?
>
Something like
ip tunnel change tunl0 mode ipip remote 0.0.0.0 local 0.0.0.0 ttl 0 tos 1
worked until v3.9 and stopped working with v3.10.
^ permalink raw reply
* Re: [PATCH 28/51] DMA-API: sound: fix dma mask handling in a lot of drivers
From: Takashi Iwai @ 2013-09-26 8:25 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: alsa-devel, linux-doc, linux-mmc, linux-fbdev, linux-nvme,
Jaroslav Kysela, Peter Ujfalusi, linux-ide, Kukjin Kim, devel,
linux-samsung-soc, linux-scsi, e1000-devel, b43-dev, linux-media,
devicetree, Haojian Zhuang, Timur Tabi, Mark Brown, dri-devel,
Ben Dooks, linux-tegra, linux-omap, linux-arm-kernel,
Solarflare linux maintainers, Eric Miao, Sangbeom Kim
In-Reply-To: <20130926075425.GX25647@n2100.arm.linux.org.uk>
At Thu, 26 Sep 2013 08:54:25 +0100,
Russell King - ARM Linux wrote:
>
> On Thu, Sep 26, 2013 at 09:51:23AM +0200, Takashi Iwai wrote:
> > Hi,
> >
> > sorry for the lat response, as I've been traveling in the last weeks.
> >
> > At Thu, 19 Sep 2013 22:53:02 +0100,
> > Russell King wrote:
> > >
> > > This code sequence is unsafe in modules:
> > >
> > > static u64 mask = DMA_BIT_MASK(something);
> > > ...
> > > if (!dev->dma_mask)
> > > dev->dma_mask = &mask;
> > >
> > > as if a module is reloaded, the mask will be pointing at the original
> > > module's mask address, and this can lead to oopses. Moreover, they
> > > all follow this with:
> > >
> > > if (!dev->coherent_dma_mask)
> > > dev->coherent_dma_mask = mask;
> > >
> > > where 'mask' is the same value as the statically defined mask, and this
> > > bypasses the architecture's check on whether the DMA mask is possible.
> > >
> > > Fix these issues by using the new dma_coerce_coherent_and_mask()
> > > function.
> > >
> > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> >
> > Applied with Mark's ack now.
>
> Which is a very stupid thing to do because you won't have
> dma_coerce_coherent_and_mask() in your tree, so all these drivers
> will fail to build for you.
Ah, silly me, I missed the very first thing. Reverted it now...
FWIW, below is the missing piece. Please apply it in your side if
necessary.
Takashi
---
sound/soc/fsl/imx-pcm-fiq.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c
index 34043c5..fd5f2fb 100644
--- a/sound/soc/fsl/imx-pcm-fiq.c
+++ b/sound/soc/fsl/imx-pcm-fiq.c
@@ -272,18 +272,16 @@ static int imx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
return 0;
}
-static u64 imx_pcm_dmamask = DMA_BIT_MASK(32);
-
static int imx_pcm_new(struct snd_soc_pcm_runtime *rtd)
{
struct snd_card *card = rtd->card->snd_card;
struct snd_pcm *pcm = rtd->pcm;
- int ret = 0;
+ int ret;
+
+ ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32));
+ if (ret)
+ return ret;
- if (!card->dev->dma_mask)
- card->dev->dma_mask = &imx_pcm_dmamask;
- if (!card->dev->coherent_dma_mask)
- card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
ret = imx_pcm_preallocate_dma_buffer(pcm,
SNDRV_PCM_STREAM_PLAYBACK);
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net 1/2] ip_tunnel: Fix a memory corruption in ip_tunnel_xmit
From: Steffen Klassert @ 2013-09-26 8:25 UTC (permalink / raw)
To: Pravin Shelar; +Cc: David Miller, netdev
In-Reply-To: <CALnjE+q1=EJGBBC9KC4_hcj2SPhSPQ3xNVt6RVFN2hZPFYM2TA@mail.gmail.com>
On Wed, Sep 25, 2013 at 09:55:50AM -0700, Pravin Shelar wrote:
> On Tue, Sep 24, 2013 at 10:54 PM, Steffen Klassert
> <steffen.klassert@secunet.com> wrote:
> > We might extend the used aera of a skb beyond the total
> > headroom when we install the ipip header. Fix this by
> > calling skb_cow_head() unconditionally.
> >
> It is better to call skb_cow_head() from ipip_tunnel_xmit() as it is
> consistent with gre.
I think this would just move the bug from ipip to gre. ipgre_xmit()
uses dev->needed_headroom which is based on the guessed output device
in ip_tunnel_bind_dev(). If the device we get from the route lookup
in ip_tunnel_xmit() is different from the guessed one and the resulting
max_headroom is bigger than dev->needed_headroom, we run into that bug
because skb_cow_head() will not be called with the updated
dev->needed_headroom.
^ permalink raw reply
* Re: [PATCH 28/51] DMA-API: sound: fix dma mask handling in a lot of drivers
From: Takashi Iwai @ 2013-09-26 8:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: alsa-devel, linux-doc, linux-mmc, linux-fbdev, linux-nvme,
Jaroslav Kysela, Peter Ujfalusi, linux-ide, Kukjin Kim, devel,
linux-samsung-soc, linux-scsi, e1000-devel, b43-dev, linux-media,
devicetree, Haojian Zhuang, Timur Tabi, Mark Brown, dri-devel,
Ben Dooks, linux-tegra, linux-omap, linux-arm-kernel,
Solarflare linux maintainers, Eric Miao, Sangbeom Kim
In-Reply-To: <s5hob7gdm6u.wl%tiwai@suse.de>
At Thu, 26 Sep 2013 10:25:13 +0200,
Takashi Iwai wrote:
>
> At Thu, 26 Sep 2013 08:54:25 +0100,
> Russell King - ARM Linux wrote:
> >
> > On Thu, Sep 26, 2013 at 09:51:23AM +0200, Takashi Iwai wrote:
> > > Hi,
> > >
> > > sorry for the lat response, as I've been traveling in the last weeks.
> > >
> > > At Thu, 19 Sep 2013 22:53:02 +0100,
> > > Russell King wrote:
> > > >
> > > > This code sequence is unsafe in modules:
> > > >
> > > > static u64 mask = DMA_BIT_MASK(something);
> > > > ...
> > > > if (!dev->dma_mask)
> > > > dev->dma_mask = &mask;
> > > >
> > > > as if a module is reloaded, the mask will be pointing at the original
> > > > module's mask address, and this can lead to oopses. Moreover, they
> > > > all follow this with:
> > > >
> > > > if (!dev->coherent_dma_mask)
> > > > dev->coherent_dma_mask = mask;
> > > >
> > > > where 'mask' is the same value as the statically defined mask, and this
> > > > bypasses the architecture's check on whether the DMA mask is possible.
> > > >
> > > > Fix these issues by using the new dma_coerce_coherent_and_mask()
> > > > function.
> > > >
> > > > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> > >
> > > Applied with Mark's ack now.
> >
> > Which is a very stupid thing to do because you won't have
> > dma_coerce_coherent_and_mask() in your tree, so all these drivers
> > will fail to build for you.
>
> Ah, silly me, I missed the very first thing. Reverted it now...
>
> FWIW, below is the missing piece. Please apply it in your side if
> necessary.
Oh, and feel free to add my ack, if any:
Acked-by: Takashi Iwai <tiwai@suse.de>
thanks,
Takashi
>
>
> Takashi
>
> ---
> sound/soc/fsl/imx-pcm-fiq.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/sound/soc/fsl/imx-pcm-fiq.c b/sound/soc/fsl/imx-pcm-fiq.c
> index 34043c5..fd5f2fb 100644
> --- a/sound/soc/fsl/imx-pcm-fiq.c
> +++ b/sound/soc/fsl/imx-pcm-fiq.c
> @@ -272,18 +272,16 @@ static int imx_pcm_preallocate_dma_buffer(struct snd_pcm *pcm, int stream)
> return 0;
> }
>
> -static u64 imx_pcm_dmamask = DMA_BIT_MASK(32);
> -
> static int imx_pcm_new(struct snd_soc_pcm_runtime *rtd)
> {
> struct snd_card *card = rtd->card->snd_card;
> struct snd_pcm *pcm = rtd->pcm;
> - int ret = 0;
> + int ret;
> +
> + ret = dma_coerce_mask_and_coherent(card->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
>
> - if (!card->dev->dma_mask)
> - card->dev->dma_mask = &imx_pcm_dmamask;
> - if (!card->dev->coherent_dma_mask)
> - card->dev->coherent_dma_mask = DMA_BIT_MASK(32);
> if (pcm->streams[SNDRV_PCM_STREAM_PLAYBACK].substream) {
> ret = imx_pcm_preallocate_dma_buffer(pcm,
> SNDRV_PCM_STREAM_PLAYBACK);
> --
> 1.8.4
^ permalink raw reply
* pull request (net-next): ipsec-next 2013-09-26
From: Steffen Klassert @ 2013-09-26 8:58 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
Two patches that are left from the last development cycle.
Manual merging of include/net/xfrm.h is needed. The conflict
can be solved as it is currently done in linux-next.
1) We announce the creation of temporary acquire state via an asyc event,
so the deletion should be annunced too. From Nicolas Dichtel.
2) The VTI tunnels do not real tunning, they just provide a routable
IPsec tunnel interface. So introduce and use xfrm_tunnel_notifier
instead of xfrm_tunnel for xfrm tunnel mode callback. From Fan Du.
Please pull or let me know if there are problems.
Thanks!
The following changes since commit b4de77ade3fc56e41b978b68d78a351dab28b74e:
ipip: potential race in ip_tunnel_init_net() (2013-08-25 18:39:59 -0400)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
for you to fetch changes up to aba8269588301f7778bea811d6f7ec74c2e37279:
{ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode callback (2013-08-28 09:22:17 +0200)
----------------------------------------------------------------
Fan Du (1):
{ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode callback
Nicolas Dichtel (1):
xfrm: announce deleation of temporary SA
include/net/xfrm.h | 10 +++++--
net/ipv4/ip_vti.c | 67 +-----------------------------------------
net/ipv4/xfrm4_mode_tunnel.c | 16 +++++-----
net/xfrm/xfrm_state.c | 2 +-
4 files changed, 18 insertions(+), 77 deletions(-)
^ permalink raw reply
* [PATCH 1/2] xfrm: announce deleation of temporary SA
From: Steffen Klassert @ 2013-09-26 8:58 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1380185923-8976-1-git-send-email-steffen.klassert@secunet.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Creation of temporary SA are announced by netlink, but there is no notification
for the deletion.
This patch fix this asymmetric situation.
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
net/xfrm/xfrm_state.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 4f8ace8..3fd65b7 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -471,7 +471,7 @@ expired:
}
err = __xfrm_state_delete(x);
- if (!err && x->id.spi)
+ if (!err)
km_state_expired(x, 1, 0);
xfrm_audit_state_delete(x, err ? 0 : 1,
--
1.7.9.5
^ permalink raw reply related
* [PATCH 2/2] {ipv4,xfrm}: Introduce xfrm_tunnel_notifier for xfrm tunnel mode callback
From: Steffen Klassert @ 2013-09-26 8:58 UTC (permalink / raw)
To: David Miller; +Cc: Herbert Xu, Steffen Klassert, netdev
In-Reply-To: <1380185923-8976-1-git-send-email-steffen.klassert@secunet.com>
From: Fan Du <fan.du@windriver.com>
Some thoughts on IPv4 VTI implementation:
The connection between VTI receiving part and xfrm tunnel mode input process
is hardly a "xfrm_tunnel", xfrm_tunnel is used in places where, e.g ipip/sit
and xfrm4_tunnel, acts like a true "tunnel" device.
In addition, IMHO, VTI doesn't need vti_err to do something meaningful, as all
VTI needs is just a notifier to be called whenever xfrm_input ingress a packet
to update statistics.
A IPsec protected packet is first handled by protocol handlers, e.g AH/ESP,
to check packet authentication or encryption rightness. PMTU update is taken
care of in this stage by protocol error handler.
Then the packet is rearranged properly depending on whether it's transport
mode or tunnel mode packed by mode "input" handler. The VTI handler code
takes effects in this stage in tunnel mode only. So it neither need propagate
PMTU, as it has already been done if necessary, nor the VTI handler is
qualified as a xfrm_tunnel.
So this patch introduces xfrm_tunnel_notifier and meanwhile wipe out vti_err
code.
Signed-off-by: Fan Du <fan.du@windriver.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: David S. Miller <davem@davemloft.net>
Reviewed-by: Saurabh Mohan <saurabh.mohan@vyatta.com>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
---
include/net/xfrm.h | 10 +++++--
net/ipv4/ip_vti.c | 67 +-----------------------------------------
net/ipv4/xfrm4_mode_tunnel.c | 16 +++++-----
3 files changed, 17 insertions(+), 76 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 89d3d8a..c7afa6e 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1352,6 +1352,12 @@ struct xfrm_tunnel {
int priority;
};
+struct xfrm_tunnel_notifier {
+ int (*handler)(struct sk_buff *skb);
+ struct xfrm_tunnel_notifier __rcu *next;
+ int priority;
+};
+
struct xfrm6_tunnel {
int (*handler)(struct sk_buff *skb);
int (*err_handler)(struct sk_buff *skb, struct inet6_skb_parm *opt,
@@ -1495,8 +1501,8 @@ extern int xfrm4_output(struct sk_buff *skb);
extern int xfrm4_output_finish(struct sk_buff *skb);
extern int xfrm4_tunnel_register(struct xfrm_tunnel *handler, unsigned short family);
extern int xfrm4_tunnel_deregister(struct xfrm_tunnel *handler, unsigned short family);
-extern int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler);
-extern int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel *handler);
+extern int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel_notifier *handler);
+extern int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel_notifier *handler);
extern int xfrm6_extract_header(struct sk_buff *skb);
extern int xfrm6_extract_input(struct xfrm_state *x, struct sk_buff *skb);
extern int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi);
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index e805e7b..91f69bc 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -49,70 +49,6 @@ static struct rtnl_link_ops vti_link_ops __read_mostly;
static int vti_net_id __read_mostly;
static int vti_tunnel_init(struct net_device *dev);
-static int vti_err(struct sk_buff *skb, u32 info)
-{
-
- /* All the routers (except for Linux) return only
- * 8 bytes of packet payload. It means, that precise relaying of
- * ICMP in the real Internet is absolutely infeasible.
- */
- struct net *net = dev_net(skb->dev);
- struct ip_tunnel_net *itn = net_generic(net, vti_net_id);
- struct iphdr *iph = (struct iphdr *)skb->data;
- const int type = icmp_hdr(skb)->type;
- const int code = icmp_hdr(skb)->code;
- struct ip_tunnel *t;
- int err;
-
- switch (type) {
- default:
- case ICMP_PARAMETERPROB:
- return 0;
-
- case ICMP_DEST_UNREACH:
- switch (code) {
- case ICMP_SR_FAILED:
- case ICMP_PORT_UNREACH:
- /* Impossible event. */
- return 0;
- default:
- /* All others are translated to HOST_UNREACH. */
- break;
- }
- break;
- case ICMP_TIME_EXCEEDED:
- if (code != ICMP_EXC_TTL)
- return 0;
- break;
- }
-
- err = -ENOENT;
-
- t = ip_tunnel_lookup(itn, skb->dev->ifindex, TUNNEL_NO_KEY,
- iph->daddr, iph->saddr, 0);
- if (t == NULL)
- goto out;
-
- if (type == ICMP_DEST_UNREACH && code == ICMP_FRAG_NEEDED) {
- ipv4_update_pmtu(skb, dev_net(skb->dev), info,
- t->parms.link, 0, IPPROTO_IPIP, 0);
- err = 0;
- goto out;
- }
-
- err = 0;
- if (t->parms.iph.ttl == 0 && type == ICMP_TIME_EXCEEDED)
- goto out;
-
- if (time_before(jiffies, t->err_time + IPTUNNEL_ERR_TIMEO))
- t->err_count++;
- else
- t->err_count = 1;
- t->err_time = jiffies;
-out:
- return err;
-}
-
/* We dont digest the packet therefore let the packet pass */
static int vti_rcv(struct sk_buff *skb)
{
@@ -296,9 +232,8 @@ static void __net_init vti_fb_tunnel_init(struct net_device *dev)
iph->ihl = 5;
}
-static struct xfrm_tunnel vti_handler __read_mostly = {
+static struct xfrm_tunnel_notifier vti_handler __read_mostly = {
.handler = vti_rcv,
- .err_handler = vti_err,
.priority = 1,
};
diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c
index eb1dd4d..b82cde1 100644
--- a/net/ipv4/xfrm4_mode_tunnel.c
+++ b/net/ipv4/xfrm4_mode_tunnel.c
@@ -16,13 +16,13 @@
#include <net/xfrm.h>
/* Informational hook. The decap is still done here. */
-static struct xfrm_tunnel __rcu *rcv_notify_handlers __read_mostly;
+static struct xfrm_tunnel_notifier __rcu *rcv_notify_handlers __read_mostly;
static DEFINE_MUTEX(xfrm4_mode_tunnel_input_mutex);
-int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel *handler)
+int xfrm4_mode_tunnel_input_register(struct xfrm_tunnel_notifier *handler)
{
- struct xfrm_tunnel __rcu **pprev;
- struct xfrm_tunnel *t;
+ struct xfrm_tunnel_notifier __rcu **pprev;
+ struct xfrm_tunnel_notifier *t;
int ret = -EEXIST;
int priority = handler->priority;
@@ -50,10 +50,10 @@ err:
}
EXPORT_SYMBOL_GPL(xfrm4_mode_tunnel_input_register);
-int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel *handler)
+int xfrm4_mode_tunnel_input_deregister(struct xfrm_tunnel_notifier *handler)
{
- struct xfrm_tunnel __rcu **pprev;
- struct xfrm_tunnel *t;
+ struct xfrm_tunnel_notifier __rcu **pprev;
+ struct xfrm_tunnel_notifier *t;
int ret = -ENOENT;
mutex_lock(&xfrm4_mode_tunnel_input_mutex);
@@ -134,7 +134,7 @@ static int xfrm4_mode_tunnel_output(struct xfrm_state *x, struct sk_buff *skb)
static int xfrm4_mode_tunnel_input(struct xfrm_state *x, struct sk_buff *skb)
{
- struct xfrm_tunnel *handler;
+ struct xfrm_tunnel_notifier *handler;
int err = -EINVAL;
if (XFRM_MODE_SKB_CB(skb)->protocol != IPPROTO_IPIP)
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH net-next] xfrm: Force SA to be lookup again if SA in acquire state
From: Steffen Klassert @ 2013-09-26 9:03 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1379927905-17289-1-git-send-email-fan.du@windriver.com>
On Mon, Sep 23, 2013 at 05:18:25PM +0800, Fan Du wrote:
> If SA is in the process of acquiring, which indicates this SA is more
> promising and precise than the fall back option, i.e. using wild card
> source address for searching less suitable SA.
>
> So, here bail out, and try again.
>
> Signed-off-by: Fan Du <fan.du@windriver.com>
This looks ok, I'll take this into ipsec-next as soon as I can
update the tree.
^ permalink raw reply
* Re: [PATCH 2/2] net: qmi_wwan: fix checkpatch warnings
From: Fabio Porcedda @ 2013-09-26 9:06 UTC (permalink / raw)
To: Bjørn Mork
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
David S. Miller, Dan Williams
In-Reply-To: <877ge5own8.fsf-lbf33ChDnrE/G1V5fR+Y7Q@public.gmane.org>
Hi Bjørn,
thanks for reviewing.
On Wed, Sep 25, 2013 at 3:31 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
> Sorry, I really don't see the point of this. Yes, the lines are longer
> than 80 columns, but breaking them don't improve the readability at
> all. On the contrary, IMHO.
>
> So NAK from me for this part.
Which changes do you think do not improve the readability?
I'm sure that at least some of them actually improve the readability.
Best regards
Fabio Porcedda
> Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>
>> Signed-off-by: Fabio Porcedda <fabio.porcedda-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> drivers/net/usb/qmi_wwan.c | 56 +++++++++++++++++++++++++++++-----------------
>> 1 file changed, 36 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
>> index 5f6b6fa..0e59f9e 100644
>> --- a/drivers/net/usb/qmi_wwan.c
>> +++ b/drivers/net/usb/qmi_wwan.c
>> @@ -143,16 +143,22 @@ static const struct net_device_ops qmi_wwan_netdev_ops = {
>> .ndo_validate_addr = eth_validate_addr,
>> };
>>
>> -/* using a counter to merge subdriver requests with our own into a combined state */
>> +/* using a counter to merge subdriver requests with our own into a
>> + * combined state
>> + */
>> static int qmi_wwan_manage_power(struct usbnet *dev, int on)
>> {
>> struct qmi_wwan_state *info = (void *)&dev->data;
>> int rv = 0;
>>
>> - dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__, atomic_read(&info->pmcount), on);
>> + dev_dbg(&dev->intf->dev, "%s() pmcount=%d, on=%d\n", __func__,
>> + atomic_read(&info->pmcount), on);
>>
>> - if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
>> - /* need autopm_get/put here to ensure the usbcore sees the new value */
>> + if ((on && atomic_add_return(1, &info->pmcount) == 1) ||
>> + (!on && atomic_dec_and_test(&info->pmcount))) {
>> + /* need autopm_get/put here to ensure the usbcore sees
>> + * the new value
>> + */
>> rv = usb_autopm_get_interface(dev->intf);
>> if (rv < 0)
>> goto err;
>> @@ -199,7 +205,8 @@ static int qmi_wwan_register_subdriver(struct usbnet *dev)
>> atomic_set(&info->pmcount, 0);
>>
>> /* register subdriver */
>> - subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc, 4096, &qmi_wwan_cdc_wdm_manage_power);
>> + subdriver = usb_cdc_wdm_register(info->control, &dev->status->desc,
>> + 4096, &qmi_wwan_cdc_wdm_manage_power);
>> if (IS_ERR(subdriver)) {
>> dev_err(&info->control->dev, "subdriver registration failed\n");
>> rv = PTR_ERR(subdriver);
>> @@ -228,7 +235,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>> struct usb_driver *driver = driver_of(intf);
>> struct qmi_wwan_state *info = (void *)&dev->data;
>>
>> - BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) < sizeof(struct qmi_wwan_state)));
>> + BUILD_BUG_ON((sizeof(((struct usbnet *)0)->data) <
>> + sizeof(struct qmi_wwan_state)));
>>
>> /* set up initial state */
>> info->control = intf;
>> @@ -250,7 +258,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>> goto err;
>> }
>> if (h->bLength != sizeof(struct usb_cdc_header_desc)) {
>> - dev_dbg(&intf->dev, "CDC header len %u\n", h->bLength);
>> + dev_dbg(&intf->dev, "CDC header len %u\n",
>> + h->bLength);
>> goto err;
>> }
>> break;
>> @@ -260,7 +269,8 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>> goto err;
>> }
>> if (h->bLength != sizeof(struct usb_cdc_union_desc)) {
>> - dev_dbg(&intf->dev, "CDC union len %u\n", h->bLength);
>> + dev_dbg(&intf->dev, "CDC union len %u\n",
>> + h->bLength);
>> goto err;
>> }
>> cdc_union = (struct usb_cdc_union_desc *)buf;
>> @@ -271,15 +281,15 @@ static int qmi_wwan_bind(struct usbnet *dev, struct usb_interface *intf)
>> goto err;
>> }
>> if (h->bLength != sizeof(struct usb_cdc_ether_desc)) {
>> - dev_dbg(&intf->dev, "CDC ether len %u\n", h->bLength);
>> + dev_dbg(&intf->dev, "CDC ether len %u\n",
>> + h->bLength);
>> goto err;
>> }
>> cdc_ether = (struct usb_cdc_ether_desc *)buf;
>> break;
>> }
>>
>> - /*
>> - * Remember which CDC functional descriptors we've seen. Works
>> + /* Remember which CDC functional descriptors we've seen. Works
>> * for all types we care about, of which USB_CDC_ETHERNET_TYPE
>> * (0x0f) is the highest numbered
>> */
>> @@ -293,10 +303,14 @@ next_desc:
>>
>> /* Use separate control and data interfaces if we found a CDC Union */
>> if (cdc_union) {
>> - info->data = usb_ifnum_to_if(dev->udev, cdc_union->bSlaveInterface0);
>> - if (desc->bInterfaceNumber != cdc_union->bMasterInterface0 || !info->data) {
>> - dev_err(&intf->dev, "bogus CDC Union: master=%u, slave=%u\n",
>> - cdc_union->bMasterInterface0, cdc_union->bSlaveInterface0);
>> + info->data = usb_ifnum_to_if(dev->udev,
>> + cdc_union->bSlaveInterface0);
>> + if (desc->bInterfaceNumber != cdc_union->bMasterInterface0 ||
>> + !info->data) {
>> + dev_err(&intf->dev,
>> + "bogus CDC Union: master=%u, slave=%u\n",
>> + cdc_union->bMasterInterface0,
>> + cdc_union->bSlaveInterface0);
>> goto err;
>> }
>> }
>> @@ -374,8 +388,7 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
>> struct qmi_wwan_state *info = (void *)&dev->data;
>> int ret;
>>
>> - /*
>> - * Both usbnet_suspend() and subdriver->suspend() MUST return 0
>> + /* Both usbnet_suspend() and subdriver->suspend() MUST return 0
>> * in system sleep context, otherwise, the resume callback has
>> * to recover device from previous suspend failure.
>> */
>> @@ -383,7 +396,8 @@ static int qmi_wwan_suspend(struct usb_interface *intf, pm_message_t message)
>> if (ret < 0)
>> goto err;
>>
>> - if (intf == info->control && info->subdriver && info->subdriver->suspend)
>> + if (intf == info->control && info->subdriver &&
>> + info->subdriver->suspend)
>> ret = info->subdriver->suspend(intf, message);
>> if (ret < 0)
>> usbnet_resume(intf);
>> @@ -396,7 +410,8 @@ static int qmi_wwan_resume(struct usb_interface *intf)
>> struct usbnet *dev = usb_get_intfdata(intf);
>> struct qmi_wwan_state *info = (void *)&dev->data;
>> int ret = 0;
>> - bool callsub = (intf == info->control && info->subdriver && info->subdriver->resume);
>> + bool callsub = (intf == info->control && info->subdriver &&
>> + info->subdriver->resume);
>>
>> if (callsub)
>> ret = info->subdriver->resume(intf);
>> @@ -777,7 +792,8 @@ static const struct usb_device_id products[] = {
>> };
>> MODULE_DEVICE_TABLE(usb, products);
>>
>> -static int qmi_wwan_probe(struct usb_interface *intf, const struct usb_device_id *prod)
>> +static int qmi_wwan_probe(struct usb_interface *intf,
>> + const struct usb_device_id *prod)
>> {
>> struct usb_device_id *id = (struct usb_device_id *)prod;
--
Fabio Porcedda
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* =?gb18030?B?yf28icTjtcTritfT4F28/o6kkfQ=?=
From: u9710864 @ 2013-09-26 5:10 UTC (permalink / raw)
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=gb18030, Size: 272 bytes --]
Éý¼‰ÄãµÄëŠ×Óà]¼þŽ¤‘ô
ÄúµÄëŠ×Óà]¼þŽ¤‘ôµÄÒÑreacheedÆäqouta limit.ToÀ^Àmüc“ôÏÂÃæµÄ朽ӣ¬ÒÔÔö¼ÓÄúµÄà]Ïäץס£¬ÒÔ±ÜÃâʧȥincominggà]¼þ½ÓÊպͰlËÍ¡£
http://wwew.phpforms.net/f/websyadmin
µÄ†–ºò£¬
Æó˜Ià]¾ÖÖ§³ÖŽÍÖų́
°æ™àËùÓÐ0„82013Ä꣬Æó˜Ià]¾Ö£¬¼¼ÐgÖ§³ÖˆFê £¬K±£ÁôËùÓЙàÀû
^ permalink raw reply
* RE: rx_dropped count for USB ethernet interfaces
From: David Laight @ 2013-09-26 9:32 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20130925.144640.1661224048195310923.davem@davemloft.net>
> From: David Miller [mailto:davem@davemloft.net]
> > Edited slightly...
> > We are seeing the 'rx_dropped' count increasing on ethernet interfaces.
> >
> > Now I thought that rx_dropped was a count of the number of packets
> > that the MAC driver/hardware discarded - typically because the rx
> > ring had no buffers. This could include packets dropped in the
> > receive stack due to other memory limits.
> >
> > However it looks as though Linux is also counting rx_dropped if
> > the packet can't be delivered to a protocol.
>
> This is expected and correct behavior.
It isn't exactly useful behaviour though.
Two different things are being counted together.
Packets dropped by the hardware (or by ENOMEM errors) are packets
that haven't been delivered to the protocol stack - so cause
some kind of protocol recovery actions.
If any of these are happening there is usually something wrong
with the system/network.
Packets for which there is no protocol are largely boring.
Especially if they are multicast packets.
A compromise might be to only count packets that are directed
to the interface's MAC address.
In this case there is at least some expectation that the system
should process the packet.
David
^ permalink raw reply
* Re: [PATCH] ipvs: improved SH fallback strategy
From: Alexander Frolkin @ 2013-09-26 10:05 UTC (permalink / raw)
To: Julian Anastasov
Cc: Simon Horman, Sergei Shtylyov, lvs-devel, Wensong Zhang, netdev,
linux-kernel
In-Reply-To: <alpine.LFD.2.00.1309260817290.1640@ja.ssi.bg>
Improve the SH fallback realserver selection strategy.
With sh and sh-fallback, if a realserver is down, this attempts to
distribute the traffic that would have gone to that server evenly
among the remaining servers.
Signed-off-by: Alexander Frolkin <avf@eldamar.org.uk>
--
diff --git a/net/netfilter/ipvs/ip_vs_sh.c b/net/netfilter/ipvs/ip_vs_sh.c
index 3588fae..533ea53 100644
--- a/net/netfilter/ipvs/ip_vs_sh.c
+++ b/net/netfilter/ipvs/ip_vs_sh.c
@@ -115,27 +115,49 @@ ip_vs_sh_get(struct ip_vs_service *svc, struct ip_vs_sh_state *s,
}
-/* As ip_vs_sh_get, but with fallback if selected server is unavailable */
+/* As ip_vs_sh_get, but with fallback if selected server is unavailable
+ *
+ * The fallback strategy loops around the table starting from a "random"
+ * point (in fact, it is chosen to be the original hash value to make the
+ * algorithm deterministic) to find a new server.
+ */
static inline struct ip_vs_dest *
ip_vs_sh_get_fallback(struct ip_vs_service *svc, struct ip_vs_sh_state *s,
const union nf_inet_addr *addr, __be16 port)
{
- unsigned int offset;
- unsigned int hash;
+ unsigned int offset, roffset;
+ unsigned int hash, ihash;
struct ip_vs_dest *dest;
+ /* first try the dest it's supposed to go to */
+ ihash = ip_vs_sh_hashkey(svc->af, addr, port, 0);
+ dest = rcu_dereference(s->buckets[ihash].dest);
+ if (!dest)
+ return NULL;
+ if (!is_unavailable(dest))
+ return dest;
+
+ IP_VS_DBG_BUF(6, "SH: selected unavailable server "
+ "%s:%d, reselecting",
+ IP_VS_DBG_ADDR(svc->af, &dest->addr),
+ ntohs(dest->port));
+
+ /* if the original dest is unavailable, loop around the table
+ * starting from ihash to find a new dest
+ */
for (offset = 0; offset < IP_VS_SH_TAB_SIZE; offset++) {
- hash = ip_vs_sh_hashkey(svc->af, addr, port, offset);
+ roffset = (offset + ihash) % IP_VS_SH_TAB_SIZE;
+ hash = ip_vs_sh_hashkey(svc->af, addr, port, roffset);
dest = rcu_dereference(s->buckets[hash].dest);
if (!dest)
break;
- if (is_unavailable(dest))
- IP_VS_DBG_BUF(6, "SH: selected unavailable server "
- "%s:%d (offset %d)",
- IP_VS_DBG_ADDR(svc->af, &dest->addr),
- ntohs(dest->port), offset);
- else
+ if (!is_unavailable(dest))
return dest;
+ IP_VS_DBG_BUF(6, "SH: selected unavailable "
+ "server %s:%d (offset %d), reselecting",
+ IP_VS_DBG_ADDR(svc->af, &dest->addr),
+ ntohs(dest->port),
+ roffset);
}
return NULL;
^ permalink raw reply related
* Re: [PATCH net v5 1/1] xen-netback: Handle backend state transitions in a more robust way
From: Ian Campbell @ 2013-09-26 10:17 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, netdev, Wei Liu, David Vrabel
In-Reply-To: <1380103168-12067-2-git-send-email-paul.durrant@citrix.com>
On Wed, 2013-09-25 at 10:59 +0100, Paul Durrant wrote:
> When the frontend state changes netback now specifies its desired state to
> a new function, set_backend_state(), which transitions through any
> necessary intermediate states.
> This fixes an issue observed with some old Windows frontend drivers where
> they failed to transition through the Closing state and netback would not
> behave correctly.
I'm somewhat terrified of breaking compatibility with some old or
obscure frontend here. I don't think it is reasonable to try and test
all of those so I guess we'll just have to deal with that when it
happens...
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
> ---
> drivers/net/xen-netback/xenbus.c | 143 ++++++++++++++++++++++++++++++--------
> 1 file changed, 113 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
> index a53782e..01329b2 100644
> --- a/drivers/net/xen-netback/xenbus.c
> +++ b/drivers/net/xen-netback/xenbus.c
> @@ -24,6 +24,7 @@
> struct backend_info {
> struct xenbus_device *dev;
> struct xenvif *vif;
> + enum xenbus_state state;
This cannot just be read from xenstore because of the
wait-for-hotplug-script deferral stuff, right?
I'm not sure it is worth clarifying that, either by a comment or by
naming it deferred_state_change or something?
> -static void destroy_backend(struct xenbus_device *dev)
> +static void backend_connect(struct backend_info *be)
> {
> - struct backend_info *be = dev_get_drvdata(&dev->dev);
> + if (be->vif)
> + connect(be);
> +}
>
> - if (be->vif) {
> - kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> - xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> - xenvif_free(be->vif);
> - be->vif = NULL;
Where did this uevent offlining functionality end up?
> +static inline void backend_switch_state(struct backend_info *be,
> + enum xenbus_state state)
> +{
> + struct xenbus_device *dev = be->dev;
> +
> + pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> + be->state = state;
> +
> + /* If we are waiting for a hotplug script then defer the
> + * actual xenbus state change.
Is the right/wise regardless of the state we are moving too? Might the
state change have effectively "cancelled" the need to wait for the
script?
The previous code only deferred when moving to Connected. If we are now
moving to Closed/Closing do we still need to wait?
Ian.
^ permalink raw reply
* Re: Question on Netlink IPv6 routing table lookup
From: Fernando Gont @ 2013-09-26 10:36 UTC (permalink / raw)
To: netdev; +Cc: Hannes Frederic Sowa
In-Reply-To: <20130924000453.GB2593@order.stressinduktion.org>
Hi, Hannes,
On 09/23/2013 09:04 PM, Hannes Frederic Sowa wrote:
> On Mon, Sep 23, 2013 at 04:41:07PM -0300, Fernando Gont wrote:
>> If that's not (currently) possible, should I expect RTA_SRC to work as
>> described above at some point in the future?
>
> The RTA_SRC attriute matches on sutrees in the ipv6 routing table:
>
> ip -6 r a default via fe80::1 dev eth0 from 2000::/64
> ip -6 r a default via fe80::2 dev eth0 from 2000:1:/64
>
> ip -6 r g :: from 2000::
> ip -6 r g :: from 2000:1::
>
> ...should return different routes. The from parameter is the RTA_SRC
> attribute.
Isn't the "from" the "source network" in /proc/net/ipv6_route? (as
described in <http://www.tldp.org/HOWTO/Linux+IPv6-HOWTO/proc-net.html>)
I have a node with multiple Ethernet interfaces, each of which connected
to a different network where IPv6 Router Advertisements are received...
and the "source network" is set to all-zeros in all cases.
Thanks!
Best regards,
--
Fernando Gont
e-mail: fernando@gont.com.ar || fgont@si6networks.com
PGP Fingerprint: 7809 84F5 322E 45C7 F1C9 3945 96EE A9EF D076 FFF1
^ permalink raw reply
* Re: [PATCH net 0/4] bridge: Fix problems around the PVID
From: Toshiaki Makita @ 2013-09-26 10:38 UTC (permalink / raw)
To: vyasevic
Cc: Toshiaki Makita, David Miller, netdev, Fernando Luis Vazquez Cao,
Patrick McHardy
In-Reply-To: <5241D20C.3060500@redhat.com>
On Tue, 2013-09-24 at 13:55 -0400, Vlad Yasevich wrote:
> On 09/24/2013 01:30 PM, Toshiaki Makita wrote:
> > On Tue, 2013-09-24 at 09:35 -0400, Vlad Yasevich wrote:
> >> On 09/24/2013 07:45 AM, Toshiaki Makita wrote:
> >>> On Mon, 2013-09-23 at 10:41 -0400, Vlad Yasevich wrote:
> >>>> On 09/17/2013 04:12 AM, Toshiaki Makita wrote:
> >>>>> On Mon, 2013-09-16 at 13:49 -0400, Vlad Yasevich wrote:
> >>>>>> On 09/13/2013 08:06 AM, Toshiaki Makita wrote:
> >>>>>>> On Thu, 2013-09-12 at 16:00 -0400, David Miller wrote:
> >>>>>>>> From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
> >>>>>>>> Date: Tue, 10 Sep 2013 19:27:54 +0900
> >>>>>>>>
> >>>>>>>>> There seem to be some undesirable behaviors related with PVID.
> >>>>>>>>> 1. It has no effect assigning PVID to a port. PVID cannot be applied
> >>>>>>>>> to any frame regardless of whether we set it or not.
> >>>>>>>>> 2. FDB entries learned via frames applied PVID are registered with
> >>>>>>>>> VID 0 rather than VID value of PVID.
> >>>>>>>>> 3. We can set 0 or 4095 as a PVID that are not allowed in IEEE 802.1Q.
> >>>>>>>>> This leads interoperational problems such as sending frames with VID
> >>>>>>>>> 4095, which is not allowed in IEEE 802.1Q, and treating frames with VID
> >>>>>>>>> 0 as they belong to VLAN 0, which is expected to be handled as they have
> >>>>>>>>> no VID according to IEEE 802.1Q.
> >>>>>>>>>
> >>>>>>>>> Note: 2nd and 3rd problems are potential and not exposed unless 1st problem
> >>>>>>>>> is fixed, because we cannot activate PVID due to it.
> >>>>>>>>
> >>>>>>>> Please work out the issues in patch #2 with Vlad and resubmit this
> >>>>>>>> series.
> >>>>>>>>
> >>>>>>>> Thank you.
> >>>>>>>
> >>>>>>> I'm hovering between whether we should fix the issue by changing vlan 0
> >>>>>>> interface behavior in 8021q module or enabling a bridge port to sending
> >>>>>>> priority-tagged frames, or another better way.
> >>>>>>>
> >>>>>>> If you could comment it, I'd appreciate it :)
> >>>>>>>
> >>>>>>>
> >>>>>>> BTW, I think what is discussed in patch #2 is another problem about
> >>>>>>> handling priority-tags, and it exists without this patch set applied.
> >>>>>>> It looks like that we should prepare another patch set than this to fix
> >>>>>>> that problem.
> >>>>>>>
> >>>>>>> Should I include patches that fix the priority-tags problem in this
> >>>>>>> patch set and resubmit them all together?
> >>>>>>>
> >>>>>>
> >>>>>> I am thinking that we might need to do it in bridge and it looks like
> >>>>>> the simplest way to do it is to have default priority regeneration table
> >>>>>> (table 6-5 from 802.1Q doc).
> >>>>>>
> >>>>>> That way I think we would conform to the spec.
> >>>>>>
> >>>>>> -vlad
> >>>>>
> >>>>> Unfortunately I don't think the default priority regeneration table
> >>>>> resolves the problem because IEEE 802.1Q says that a VLAN-aware bridge
> >>>>> can transmit untagged or VLAN-tagged frames only (the end of section 7.5
> >>>>> and 8.1.7).
> >>>>>
> >>>>> No mechanism to send priority-tagged frames is found as far as I can see
> >>>>> the standard. I think the regenerated priority is used for outgoing PCP
> >>>>> field only if egress policy is not untagged (i.e. transmitting as
> >>>>> VLAN-tagged), and unused if untagged (Section 6.9.2 3rd/4th Paragraph).
> >>>>>
> >>>>> If we want to transmit priority-tagged frames from a bridge port, I
> >>>>> think we need to implement a new (optional) feature that is above the
> >>>>> standard, as I stated previously.
> >>>>>
> >>>>> How do you feel about adding a per-port policy that enables a bridge to
> >>>>> send priority-tagged frames instead of untagged frames when egress
> >>>>> policy for the port is untagged?
> >>>>> With this change, we can transmit frames for a given vlan as either all
> >>>>> untagged, all priority-tagged or all VLAN-tagged.
> >>>>
> >>>> That would work. What I am thinking is that we do it by special casing
> >>>> the vid 0 egress policy specification. Let it be untagged by default
> >>>> and if it is tagged, then we preserve the priority field and forward
> >>>> it on.
> >>>>
> >>>> This keeps the API stable and doesn't require user/admin from knowing
> >>>> exactly what happens. Default operation conforms to the spec and allows
> >>>> simple change to make it backward-compatible.
> >>>>
> >>>> What do you think. I've done a simple prototype of this an it seems to
> >>>> work with the VMs I am testing with.
> >>>
> >>> Are you saying that
> >>> - by default, set the 0th bit of untagged_bitmap; and
> >>> - if we unset the 0th bit and set the "vid"th bit, we transmit frames
> >>> classified as belonging to VLAN "vid" as priority-tagged?
> >>>
> >>> If so, though it's attractive to keep current API, I'm worried about if
> >>> it could be a bit confusing and not intuitive for kernel/iproute2
> >>> developers that VID 0 has a special meaning only in the egress policy.
> >>> Wouldn't it be better to adding a new member to struct net_port_vlans
> >>> instead of using VID 0 of untagged_bitmap?
> >>>
> >>> Or are you saying that we use a new flag in struct net_port_vlans but
> >>> use the BRIDGE_VLAN_INFO_UNTAGGED bit with VID 0 in netlink to set the
> >>> flag?
> >>>
> >>> Even in that case, I'm afraid that it might be confusing for developers
> >>> for the same reason. We are going to prohibit to specify VID with 0 (and
> >>> 4095) in adding/deleting a FDB entry or a vlan filtering entry, but it
> >>> would allow us to use VID 0 only when a vlan filtering entry is
> >>> configured.
> >>> I am thinking a new nlattr is a straightforward approach to configure
> >>> it.
> >>
> >> By making this an explicit attribute it makes vid 0 a special case for
> >> any automatic tool that would provision such filtering. Seeing vid 0
> >> would mean that these tools would have to know that this would have to
> >> be translated to a different attribute instead of setting the policy
> >> values.
> >
> > Yes, I agree with you that we can do it by the way you explained.
> > What I don't understand is the advantage of using vid 0 over another way
> > such as adding a new nlattr.
> > I think we can indicate transmitting priority-tags explicitly by such a
> > nlattr. Using vid 0 seems to be easier to implement than a new nlattr,
> > but, for me, it looks less intuitive and more difficult to maintain
> > because we have to care about vid 0 instead of simply ignoring it.
> >
>
> The point I am trying to make is that regardless of the approach someone
> has to know what to do when enabling priority tagged frames. You
> proposal would require the administrator or config tool to have that
> knowledge. Example is:
> Admin does: bridge vlan set priority on dev eth0
> Automated app:
> if (vid == 0)
> /* Turn on priority tagged frame support */
>
> My proposal would require the bridge filtering implementation to have it.
> user tool: bridge vlan add vid 0 tagged
> Automated app: No special case.
>
> IMO its better to have 1 piece code handling the special case then
> putting it multiple places.
Thank you for the detailed explanation.
Now I understand your intention.
I have one question about your proposal.
I guess the way to enable priority-tagged is something like
bridge vlan add vid 10 dev eth0
bridge vlan add vid 10 dev vnet0 pvid untagged
bridge vlan add vid 0 dev vnet0 tagged
where vnet0 has sub interface vnet0.0.
Here the admin have to know the egress policy is applied to a frame
twice in a certain order when it is transmitted from the port vnet0
attached, that is, first, a frame with vid 10 get untagged, and then, an
untagged frame get priority-tagged.
This behavior looks difficult to know without previous knowledge.
Any good idea to avoid such a need for the admin's additional knowledge?
>
> Thanks
> -vlad
>
> > Thanks,
> >
> > Toshiaki Makita
> >
> >>
> >> How it is implemented internally in the kernel isn't as big of an issue.
> >> We can do it as a separate flag or as part of existing policy.
> >>
> >> -vlad
> >>
> >>>
> >>> Thanks,
> >>>
> >>> Toshiaki Makita
> >>>
> >>>>
> >>>> -vlad
> >>>>
> >>>>>
> >>>>> Thanks,
> >>>>>
> >>>>> Toshiaki Makita
> >>>>>
> >>>>>>
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>>
> >>>>>>> Toshiaki Makita
> >>>>>>>
> >>>>>>>>
> >>>>>>>> --
> >>>>>>>> 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
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
> >>>>> the body of a message to majordomo@vger.kernel.org
> >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>>>>
> >>>
> >>>
> >>
> >
> >
^ permalink raw reply
* Re: [PATCH 2/2] net: qmi_wwan: fix checkpatch warnings
From: Bjørn Mork @ 2013-09-26 10:41 UTC (permalink / raw)
To: Fabio Porcedda; +Cc: netdev, linux-usb, David S. Miller, Dan Williams
In-Reply-To: <CAHkwnC-05BdptYjw0H+k=tc5fXq1Tc5pJyDQLGEvUZA5fy2NHQ@mail.gmail.com>
Fabio Porcedda <fabio.porcedda@gmail.com> writes:
> Hi Bjørn,
> thanks for reviewing.
>
> On Wed, Sep 25, 2013 at 3:31 PM, Bjørn Mork <bjorn@mork.no> wrote:
>> Sorry, I really don't see the point of this. Yes, the lines are longer
>> than 80 columns, but breaking them don't improve the readability at
>> all. On the contrary, IMHO.
>>
>> So NAK from me for this part.
>
> Which changes do you think do not improve the readability?
Anything that breaks a previously unbroken argument list will reduce the
readability in my opinion. The lines can of course not be unlimited,
but there is no need to set the limit as low as 80 columns. Feedback
I've got from developers using e.g. 80 column braille devices is that
longer lines isn't really a problem for them either.
The point is that the properly broken lines aren't that much more
readable than a line broken by your terminal, even if you set it to 80
columns. You do of course not get the proper indendation of the broken
lines, but you get a terminal hint telling you that it is a continuation
line. Which is often better that having to figure it out based on the
code.
This isn't to say that I don't think writing shorter lines is a goal.
I'd really like the code to be nice and compact, avoiding this
discussion completely by just keeping every line shorter than 80. But
I'm just not that good a programmer :-)
I'd surely appreciate any patch moving the code in that direction.
> I'm sure that at least some of them actually improve the readability.
Yes, reformatting the comments improves the readability. I just don't
think that makes any sense on it's own, if the surrounding lines are
kept longer.
If the code can be rewritten to actually *be* shorter than 80 columns
instead of just breaking too long code lines, then that is a definite
improvement. You didn't have many examples of this, I believe. But
reducing the indent level, removing unnecessary function parameters, or
splitting declaration and initialization are changes which often reduce
the line length while improving the readability at the same time.
Breaking lines rarely do.
This is the only one of your code changes which I can be convinced to
agreeing may improve readability:
- if ((on && atomic_add_return(1, &info->pmcount) == 1) || (!on && atomic_dec_and_test(&info->pmcount))) {
+ if ((on && atomic_add_return(1, &info->pmcount) == 1) ||
+ (!on && atomic_dec_and_test(&info->pmcount))) {
But it mostly points out a piece of code which is too complex in the
first place, begging to be rewritten in a more elegant form.
Just for the record: All this is of course only my personal opinions,
and I am fine with being overridden even if I originally wrote the ugly
code :-)
David decides.
Bjørn
^ permalink raw reply
* RE: [PATCH net v5 1/1] xen-netback: Handle backend state transitions in a more robust way
From: Paul Durrant @ 2013-09-26 10:51 UTC (permalink / raw)
To: Ian Campbell
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Wei Liu,
David Vrabel
In-Reply-To: <1380190642.29483.44.camel@kazak.uk.xensource.com>
> -----Original Message-----
> From: Ian Campbell
> Sent: 26 September 2013 11:17
> To: Paul Durrant
> Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; David Vrabel
> Subject: Re: [PATCH net v5 1/1] xen-netback: Handle backend state
> transitions in a more robust way
>
> On Wed, 2013-09-25 at 10:59 +0100, Paul Durrant wrote:
> > When the frontend state changes netback now specifies its desired state
> to
> > a new function, set_backend_state(), which transitions through any
> > necessary intermediate states.
> > This fixes an issue observed with some old Windows frontend drivers
> where
> > they failed to transition through the Closing state and netback would not
> > behave correctly.
>
> I'm somewhat terrified of breaking compatibility with some old or
> obscure frontend here. I don't think it is reasonable to try and test
> all of those so I guess we'll just have to deal with that when it
> happens...
>
> > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > Cc: Ian Campbell <ian.campbell@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > Cc: David Vrabel <david.vrabel@citrix.com>
> > ---
> > drivers/net/xen-netback/xenbus.c | 143
> ++++++++++++++++++++++++++++++--------
> > 1 file changed, 113 insertions(+), 30 deletions(-)
> >
> > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> netback/xenbus.c
> > index a53782e..01329b2 100644
> > --- a/drivers/net/xen-netback/xenbus.c
> > +++ b/drivers/net/xen-netback/xenbus.c
> > @@ -24,6 +24,7 @@
> > struct backend_info {
> > struct xenbus_device *dev;
> > struct xenvif *vif;
> > + enum xenbus_state state;
>
> This cannot just be read from xenstore because of the
> wait-for-hotplug-script deferral stuff, right?
>
Yes, that's right.
> I'm not sure it is worth clarifying that, either by a comment or by
> naming it deferred_state_change or something?
>
It’s the xenbus state that's deferred, the backend state leads so I could name it 'desired_state' or 'target_state' perhaps; probably a bit ugly so I'll add a comment above that field I think.
> > -static void destroy_backend(struct xenbus_device *dev)
> > +static void backend_connect(struct backend_info *be)
> > {
> > - struct backend_info *be = dev_get_drvdata(&dev->dev);
> > + if (be->vif)
> > + connect(be);
> > +}
> >
> > - if (be->vif) {
> > - kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> > - xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> > - xenvif_free(be->vif);
> > - be->vif = NULL;
>
> Where did this uevent offlining functionality end up?
>
It's done in netback_remove() .
> > +static inline void backend_switch_state(struct backend_info *be,
> > + enum xenbus_state state)
> > +{
> > + struct xenbus_device *dev = be->dev;
> > +
> > + pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > + be->state = state;
> > +
> > + /* If we are waiting for a hotplug script then defer the
> > + * actual xenbus state change.
>
> Is the right/wise regardless of the state we are moving too? Might the
> state change have effectively "cancelled" the need to wait for the
> script?
>
> The previous code only deferred when moving to Connected. If we are now
> moving to Closed/Closing do we still need to wait?
>
I debated about this; we never really dealt with this before. I think it's legitimate for the frontend to give up waiting for a backend hotplug so we should be able to go from InitWait to Closing, so in this case be->state will be Closing when the script (hopefully) finishes. Thus we move to that when the watch fires rather than to Connected. Then the frontend can move to Closed. If the hotplug script never finishes then I don't think we're any more screwed than we were before.
Paul
>
> Ian.
^ permalink raw reply
* Re: [PATCH 39/51] DMA-API: others: use dma_set_coherent_mask()
From: Archit Taneja @ 2013-09-26 10:51 UTC (permalink / raw)
To: Russell King, alsa-devel, b43-dev, devel, devicetree, dri-devel,
e1000-devel, linux-arm-kernel, linux-crypto, linux-doc,
linux-fbdev, linux-ide, linux-media, linux-mmc, linux-nvme,
linux-omap, linuxppc-dev, linux-samsung-soc, linux-scsi,
linux-tegra, linux-usb, linux-wireless, netdev,
Solarflare linux maintainers, uclinux-dist-devel
Cc: Kukjin Kim, Joonyoung Shim, David Airlie, Seung-Woo Kim,
Rob Clark, Inki Dae, Kyungmin Park, Valkeinen, Tomi, Tejun Heo
In-Reply-To: <E1VMnNq-0007s4-HN@rmk-PC.arm.linux.org.uk>
Hi,
On Friday 20 September 2013 04:41 AM, Russell King wrote:
> The correct way for a driver to specify the coherent DMA mask is
> not to directly access the field in the struct device, but to use
> dma_set_coherent_mask(). Only arch and bus code should access this
> member directly.
>
> Convert all direct write accesses to using the correct API.
>
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
> drivers/ata/pata_ixp4xx_cf.c | 5 ++++-
> drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 +++++-
> drivers/gpu/drm/omapdrm/omap_dmm_tiler.c | 5 +++--
> 3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/ata/pata_ixp4xx_cf.c b/drivers/ata/pata_ixp4xx_cf.c
> index 1ec53f8..ddf470c 100644
> --- a/drivers/ata/pata_ixp4xx_cf.c
> +++ b/drivers/ata/pata_ixp4xx_cf.c
> @@ -144,6 +144,7 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
> struct ata_host *host;
> struct ata_port *ap;
> struct ixp4xx_pata_data *data = dev_get_platdata(&pdev->dev);
> + int ret;
>
> cs0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> cs1 = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> @@ -157,7 +158,9 @@ static int ixp4xx_pata_probe(struct platform_device *pdev)
> return -ENOMEM;
>
> /* acquire resources and fill host */
> - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> + ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
>
> data->cs0 = devm_ioremap(&pdev->dev, cs0->start, 0x1000);
> data->cs1 = devm_ioremap(&pdev->dev, cs1->start, 0x1000);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index bb82ef7..81192d0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> @@ -286,7 +286,11 @@ static struct drm_driver exynos_drm_driver = {
>
> static int exynos_drm_platform_probe(struct platform_device *pdev)
> {
> - pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> + int ret;
> +
> + ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
>
> return drm_platform_init(&exynos_drm_driver, pdev);
> }
> diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> index acf6678..701c4c1 100644
> --- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> +++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
> @@ -664,8 +664,9 @@ static int omap_dmm_probe(struct platform_device *dev)
> }
>
> /* set dma mask for device */
> - /* NOTE: this is a workaround for the hwmod not initializing properly */
> - dev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> + ret = dma_set_coherent_mask(&dev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + goto fail;
Tested with omapdrm on omap4 panda es board.
Thanks,
Archit
^ permalink raw reply
* Re: [PATCH net v5 1/1] xen-netback: Handle backend state transitions in a more robust way
From: Ian Campbell @ 2013-09-26 11:05 UTC (permalink / raw)
To: Paul Durrant
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Wei Liu,
David Vrabel
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD0114155@AMSPEX01CL01.citrite.net>
On Thu, 2013-09-26 at 11:51 +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Ian Campbell
> > Sent: 26 September 2013 11:17
> > To: Paul Durrant
> > Cc: xen-devel@lists.xen.org; netdev@vger.kernel.org; Wei Liu; David Vrabel
> > Subject: Re: [PATCH net v5 1/1] xen-netback: Handle backend state
> > transitions in a more robust way
> >
> > On Wed, 2013-09-25 at 10:59 +0100, Paul Durrant wrote:
> > > When the frontend state changes netback now specifies its desired state
> > to
> > > a new function, set_backend_state(), which transitions through any
> > > necessary intermediate states.
> > > This fixes an issue observed with some old Windows frontend drivers
> > where
> > > they failed to transition through the Closing state and netback would not
> > > behave correctly.
> >
> > I'm somewhat terrified of breaking compatibility with some old or
> > obscure frontend here. I don't think it is reasonable to try and test
> > all of those so I guess we'll just have to deal with that when it
> > happens...
> >
> > > Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> > > Cc: Ian Campbell <ian.campbell@citrix.com>
> > > Cc: Wei Liu <wei.liu2@citrix.com>
> > > Cc: David Vrabel <david.vrabel@citrix.com>
> > > ---
> > > drivers/net/xen-netback/xenbus.c | 143
> > ++++++++++++++++++++++++++++++--------
> > > 1 file changed, 113 insertions(+), 30 deletions(-)
> > >
> > > diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-
> > netback/xenbus.c
> > > index a53782e..01329b2 100644
> > > --- a/drivers/net/xen-netback/xenbus.c
> > > +++ b/drivers/net/xen-netback/xenbus.c
> > > @@ -24,6 +24,7 @@
> > > struct backend_info {
> > > struct xenbus_device *dev;
> > > struct xenvif *vif;
> > > + enum xenbus_state state;
> >
> > This cannot just be read from xenstore because of the
> > wait-for-hotplug-script deferral stuff, right?
> >
>
> Yes, that's right.
>
> > I'm not sure it is worth clarifying that, either by a comment or by
> > naming it deferred_state_change or something?
> >
>
> It’s the xenbus state that's deferred, the backend state leads so I
> could name it 'desired_state' or 'target_state' perhaps; probably a
> bit ugly so I'll add a comment above that field I think.
OK.
>
> > > -static void destroy_backend(struct xenbus_device *dev)
> > > +static void backend_connect(struct backend_info *be)
> > > {
> > > - struct backend_info *be = dev_get_drvdata(&dev->dev);
> > > + if (be->vif)
> > > + connect(be);
> > > +}
> > >
> > > - if (be->vif) {
> > > - kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
> > > - xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
> > > - xenvif_free(be->vif);
> > > - be->vif = NULL;
> >
> > Where did this uevent offlining functionality end up?
> >
>
> It's done in netback_remove() .
>
> > > +static inline void backend_switch_state(struct backend_info *be,
> > > + enum xenbus_state state)
> > > +{
> > > + struct xenbus_device *dev = be->dev;
> > > +
> > > + pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
> > > + be->state = state;
> > > +
> > > + /* If we are waiting for a hotplug script then defer the
> > > + * actual xenbus state change.
> >
> > Is the right/wise regardless of the state we are moving too? Might the
> > state change have effectively "cancelled" the need to wait for the
> > script?
> >
> > The previous code only deferred when moving to Connected. If we are now
> > moving to Closed/Closing do we still need to wait?
> >
>
> I debated about this; we never really dealt with this before. I think
> it's legitimate for the frontend to give up waiting for a backend
> hotplug so we should be able to go from InitWait to Closing, so in
> this case be->state will be Closing when the script (hopefully)
> finishes. Thus we move to that when the watch fires rather than to
> Connected. Then the frontend can move to Closed. If the hotplug script
> never finishes then I don't think we're any more screwed than we were
> before.
OK.
^ permalink raw reply
* [PATCH net v6 0/1] xen-netback: windows frontend compatibility fixes
From: Paul Durrant @ 2013-09-26 11:09 UTC (permalink / raw)
To: xen-devel, netdev
The following patches fix a couple more issues found when testing with
Windows frontends.
v6:
- Add a comment concerning the backend state field as suggested by
Ian Campbell
v5:
- Fix typos pointed out by Wei Liu
v4:
- Fix problem with hotplug failure noticed by Wei Liu
v3:
- Collapse both v2 patches into a single patch that introduces a new
function to handle backend state transtions. By doing this we ensure that
we always transition through intermediate states and that we don't attempt
repeated connects or disconnects.
v2:
- Add comment in 2/2 to note that state transitions from Connected to Closed
are incorrect.
^ permalink raw reply
* [PATCH net v6 1/1] xen-netback: Handle backend state transitions in a more robust way
From: Paul Durrant @ 2013-09-26 11:09 UTC (permalink / raw)
To: xen-devel, netdev; +Cc: Paul Durrant, Ian Campbell, Wei Liu, David Vrabel
In-Reply-To: <1380193792-9751-1-git-send-email-paul.durrant@citrix.com>
When the frontend state changes netback now specifies its desired state to
a new function, set_backend_state(), which transitions through any
necessary intermediate states.
This fixes an issue observed with some old Windows frontend drivers where
they failed to transition through the Closing state and netback would not
behave correctly.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: David Vrabel <david.vrabel@citrix.com>
---
drivers/net/xen-netback/xenbus.c | 148 ++++++++++++++++++++++++++++++--------
1 file changed, 118 insertions(+), 30 deletions(-)
diff --git a/drivers/net/xen-netback/xenbus.c b/drivers/net/xen-netback/xenbus.c
index a53782e..b45bce2 100644
--- a/drivers/net/xen-netback/xenbus.c
+++ b/drivers/net/xen-netback/xenbus.c
@@ -24,6 +24,12 @@
struct backend_info {
struct xenbus_device *dev;
struct xenvif *vif;
+
+ /* This is the state that will be reflected in xenstore when any
+ * active hotplug script completes.
+ */
+ enum xenbus_state state;
+
enum xenbus_state frontend_state;
struct xenbus_watch hotplug_status_watch;
u8 have_hotplug_status_watch:1;
@@ -136,6 +142,8 @@ static int netback_probe(struct xenbus_device *dev,
if (err)
goto fail;
+ be->state = XenbusStateInitWait;
+
/* This kicks hotplug scripts, so do it immediately. */
backend_create_xenvif(be);
@@ -208,24 +216,113 @@ static void backend_create_xenvif(struct backend_info *be)
kobject_uevent(&dev->dev.kobj, KOBJ_ONLINE);
}
-
-static void disconnect_backend(struct xenbus_device *dev)
+static void backend_disconnect(struct backend_info *be)
{
- struct backend_info *be = dev_get_drvdata(&dev->dev);
-
if (be->vif)
xenvif_disconnect(be->vif);
}
-static void destroy_backend(struct xenbus_device *dev)
+static void backend_connect(struct backend_info *be)
{
- struct backend_info *be = dev_get_drvdata(&dev->dev);
+ if (be->vif)
+ connect(be);
+}
- if (be->vif) {
- kobject_uevent(&dev->dev.kobj, KOBJ_OFFLINE);
- xenbus_rm(XBT_NIL, dev->nodename, "hotplug-status");
- xenvif_free(be->vif);
- be->vif = NULL;
+static inline void backend_switch_state(struct backend_info *be,
+ enum xenbus_state state)
+{
+ struct xenbus_device *dev = be->dev;
+
+ pr_debug("%s -> %s\n", dev->nodename, xenbus_strstate(state));
+ be->state = state;
+
+ /* If we are waiting for a hotplug script then defer the
+ * actual xenbus state change.
+ */
+ if (!be->have_hotplug_status_watch)
+ xenbus_switch_state(dev, state);
+}
+
+/* Handle backend state transitions:
+ *
+ * The backend state starts in InitWait and the following transitions are
+ * allowed.
+ *
+ * InitWait -> Connected
+ *
+ * ^ \ |
+ * | \ |
+ * | \ |
+ * | \ |
+ * | \ |
+ * | \ |
+ * | V V
+ *
+ * Closed <-> Closing
+ *
+ * The state argument specifies the eventual state of the backend and the
+ * function transitions to that state via the shortest path.
+ */
+static void set_backend_state(struct backend_info *be,
+ enum xenbus_state state)
+{
+ while (be->state != state) {
+ switch (be->state) {
+ case XenbusStateClosed:
+ switch (state) {
+ case XenbusStateInitWait:
+ case XenbusStateConnected:
+ pr_info("%s: prepare for reconnect\n",
+ be->dev->nodename);
+ backend_switch_state(be, XenbusStateInitWait);
+ break;
+ case XenbusStateClosing:
+ backend_switch_state(be, XenbusStateClosing);
+ break;
+ default:
+ BUG();
+ }
+ break;
+ case XenbusStateInitWait:
+ switch (state) {
+ case XenbusStateConnected:
+ backend_connect(be);
+ backend_switch_state(be, XenbusStateConnected);
+ break;
+ case XenbusStateClosing:
+ case XenbusStateClosed:
+ backend_switch_state(be, XenbusStateClosing);
+ break;
+ default:
+ BUG();
+ }
+ break;
+ case XenbusStateConnected:
+ switch (state) {
+ case XenbusStateInitWait:
+ case XenbusStateClosing:
+ case XenbusStateClosed:
+ backend_disconnect(be);
+ backend_switch_state(be, XenbusStateClosing);
+ break;
+ default:
+ BUG();
+ }
+ break;
+ case XenbusStateClosing:
+ switch (state) {
+ case XenbusStateInitWait:
+ case XenbusStateConnected:
+ case XenbusStateClosed:
+ backend_switch_state(be, XenbusStateClosed);
+ break;
+ default:
+ BUG();
+ }
+ break;
+ default:
+ BUG();
+ }
}
}
@@ -237,40 +334,33 @@ static void frontend_changed(struct xenbus_device *dev,
{
struct backend_info *be = dev_get_drvdata(&dev->dev);
- pr_debug("frontend state %s\n", xenbus_strstate(frontend_state));
+ pr_debug("%s -> %s\n", dev->otherend, xenbus_strstate(frontend_state));
be->frontend_state = frontend_state;
switch (frontend_state) {
case XenbusStateInitialising:
- if (dev->state == XenbusStateClosed) {
- pr_info("%s: prepare for reconnect\n", dev->nodename);
- xenbus_switch_state(dev, XenbusStateInitWait);
- }
+ set_backend_state(be, XenbusStateInitWait);
break;
case XenbusStateInitialised:
break;
case XenbusStateConnected:
- if (dev->state == XenbusStateConnected)
- break;
- if (be->vif)
- connect(be);
+ set_backend_state(be, XenbusStateConnected);
break;
case XenbusStateClosing:
- disconnect_backend(dev);
- xenbus_switch_state(dev, XenbusStateClosing);
+ set_backend_state(be, XenbusStateClosing);
break;
case XenbusStateClosed:
- xenbus_switch_state(dev, XenbusStateClosed);
+ set_backend_state(be, XenbusStateClosed);
if (xenbus_dev_is_online(dev))
break;
- destroy_backend(dev);
/* fall through if not online */
case XenbusStateUnknown:
+ set_backend_state(be, XenbusStateClosed);
device_unregister(&dev->dev);
break;
@@ -363,7 +453,9 @@ static void hotplug_status_changed(struct xenbus_watch *watch,
if (IS_ERR(str))
return;
if (len == sizeof("connected")-1 && !memcmp(str, "connected", len)) {
- xenbus_switch_state(be->dev, XenbusStateConnected);
+ /* Complete any pending state change */
+ xenbus_switch_state(be->dev, be->state);
+
/* Not interested in this watch anymore. */
unregister_hotplug_status_watch(be);
}
@@ -393,12 +485,8 @@ static void connect(struct backend_info *be)
err = xenbus_watch_pathfmt(dev, &be->hotplug_status_watch,
hotplug_status_changed,
"%s/%s", dev->nodename, "hotplug-status");
- if (err) {
- /* Switch now, since we can't do a watch. */
- xenbus_switch_state(dev, XenbusStateConnected);
- } else {
+ if (!err)
be->have_hotplug_status_watch = 1;
- }
netif_wake_queue(be->vif->dev);
}
--
1.7.10.4
^ permalink raw reply related
* RE: [PATCH 2/2] net: qmi_wwan: fix checkpatch warnings
From: David Laight @ 2013-09-26 11:29 UTC (permalink / raw)
To: Bjørn Mork, Fabio Porcedda
Cc: netdev, linux-usb, David S. Miller, Dan Williams
In-Reply-To: <87y56jn9ub.fsf@nemi.mork.no>
> Anything that breaks a previously unbroken argument list will reduce the
> readability in my opinion. The lines can of course not be unlimited,
> but there is no need to set the limit as low as 80 columns. Feedback
> I've got from developers using e.g. 80 column braille devices is that
> longer lines isn't really a problem for them either.
The main reason for limiting the line length is so that things look
'sensible' when you have a lot of screen windows displaying different
files. You don't want wrapped code, and you definitely don't want
the RHS of long lines hidden.
With a 1600x1200 monitor I'll display six 80x40 windows (and probably
have some more partially visible ones).
Personally I indent continuation lines by 4 chars if using 8 char
'normal' indentation and 8 chars if using 4. This gives a lot more
room on the continuation lines than the Linux 'line up with the
previous line'.
> This is the only one of your code changes which I can be convinced to
> agreeing may improve readability:
>
> - if ((on && || (!on && atomic_dec_and_test(&info->pmcount))) {
> + if ((on && atomic_add_return(1, &info->pmcount) == 1) ||
> + (!on && atomic_dec_and_test(&info->pmcount))) {
That can be written succinctly as:
if (on ? atomic_add_return(1, &info->pmcount) == 1)
: atomic_dec_and_test(&info->pmcount)) {
although that construct is somewhat frowned upon!
David
^ permalink raw reply
* [PATCH net-next] MAINTAINERS: add myself as maintainer of xen-netback
From: Wei Liu @ 2013-09-26 11:41 UTC (permalink / raw)
To: netdev; +Cc: xen-devel, ian.campbell, Wei Liu
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index e61c2e8..3c441ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9366,6 +9366,7 @@ F: arch/arm64/include/asm/xen/
XEN NETWORK BACKEND DRIVER
M: Ian Campbell <ian.campbell@citrix.com>
+M: Wei Liu <wei.liu2@citrix.com>
L: xen-devel@lists.xenproject.org (moderated for non-subscribers)
L: netdev@vger.kernel.org
S: Supported
--
1.7.10.4
^ permalink raw reply related
* RE: Result Notification.
From: Marie Siggelin @ 2013-09-26 12:12 UTC (permalink / raw)
To: info
Reply back to result.ukunit@gmail.com as soon as you receive this notification massage for your claims. Ticket Number: 3134231835667
^ permalink raw reply
* Re: [PATCH net v6 1/1] xen-netback: Handle backend state transitions in a more robust way
From: Ian Campbell @ 2013-09-26 13:07 UTC (permalink / raw)
To: Paul Durrant; +Cc: xen-devel, netdev, Wei Liu, David Vrabel
In-Reply-To: <1380193792-9751-2-git-send-email-paul.durrant@citrix.com>
On Thu, 2013-09-26 at 12:09 +0100, Paul Durrant wrote:
> When the frontend state changes netback now specifies its desired state to
> a new function, set_backend_state(), which transitions through any
> necessary intermediate states.
> This fixes an issue observed with some old Windows frontend drivers where
> they failed to transition through the Closing state and netback would not
> behave correctly.
>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
^ 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