* Re: [PATCH net] tun: remove bogus hardware vlan acceleration flags from vlan_features
From: David Miller @ 2014-02-20 7:16 UTC (permalink / raw)
To: makita.toshiaki; +Cc: fernando, maxk, netdev
In-Reply-To: <1392726009-8083-2-git-send-email-makita.toshiaki@lab.ntt.co.jp>
From: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Date: Tue, 18 Feb 2014 21:20:09 +0900
> From: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
>
> Even though only the outer vlan tag can be HW accelerated in the transmission
> path, in the TUN/TAP driver vlan_features mirrors hw_features, which happens
> to have the NETIF_F_HW_VLAN_?TAG_TX flags set. Because of this, during packet
> tranmisssion through a stacked vlan device dev_hard_start_xmit, (incorrectly)
> assuming that the vlan device supports hardware vlan acceleration, does not
> add the vlan header to the skb payload and the inner vlan tags are lost
> (vlan_tci contains the outer vlan tag when userspace reads the packet from
> the tap device).
>
> Signed-off-by: Fernando Luis Vazquez Cao <fernando@oss.ntt.co.jp>
> Signed-off-by: Toshiaki Makita <makita.toshiaki@lab.ntt.co.jp>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH RFC net-next 0/6] 6lowpan: reimplementation of fragmentation handling
From: Alexander Aring @ 2014-02-20 8:46 UTC (permalink / raw)
To: davem
Cc: dbaryshkov, alex.bluesman.smirnov, linux-zigbee-devel, netdev,
martin.townsend
In-Reply-To: <20140218195824.GA935@omega>
Dear David Miller,
looks this patch stack okay for you?
I don't want to be annoying, I worked a long time on these series and I would
be happy if these patches will apply into mainline. Then I can take the
next TODO on my long "6lowpan/ieee802154" list.
- Alex
^ permalink raw reply
* [PATCH net v2] sit: fix panic with route cache in ip tunnels
From: Nicolas Dichtel @ 2014-02-20 9:19 UTC (permalink / raw)
To: davem; +Cc: netdev, eric.dumazet, Nicolas Dichtel, Tom Herbert
In-Reply-To: <5304DA44.2090305@6wind.com>
Bug introduced by commit 7d442fab0a67 ("ipv4: Cache dst in tunnels").
Because sit code does not call ip_tunnel_init(), the dst_cache was not
initialized.
CC: Tom Herbert <therbert@google.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
v2: fix dst release
fix fb dev init
include/net/ip_tunnels.h | 1 +
net/ipv4/ip_tunnel.c | 7 ++++---
net/ipv6/sit.c | 19 +++++++++++++++++++
3 files changed, 24 insertions(+), 3 deletions(-)
diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 48ed75c21260..e77c10405d51 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -129,6 +129,7 @@ int ip_tunnel_changelink(struct net_device *dev, struct nlattr *tb[],
int ip_tunnel_newlink(struct net_device *dev, struct nlattr *tb[],
struct ip_tunnel_parm *p);
void ip_tunnel_setup(struct net_device *dev, int net_id);
+void ip_tunnel_dst_reset_all(struct ip_tunnel *t);
/* Extract dsfield from inner protocol */
static inline u8 ip_tunnel_get_dsfield(const struct iphdr *iph,
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 50228be5c17b..74df7f12d76e 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -93,13 +93,14 @@ static void tunnel_dst_reset(struct ip_tunnel *t)
tunnel_dst_set(t, NULL);
}
-static void tunnel_dst_reset_all(struct ip_tunnel *t)
+void ip_tunnel_dst_reset_all(struct ip_tunnel *t)
{
int i;
for_each_possible_cpu(i)
__tunnel_dst_set(per_cpu_ptr(t->dst_cache, i), NULL);
}
+EXPORT_SYMBOL(ip_tunnel_dst_reset_all);
static struct rtable *tunnel_rtable_get(struct ip_tunnel *t, u32 cookie)
{
@@ -759,7 +760,7 @@ static void ip_tunnel_update(struct ip_tunnel_net *itn,
if (set_mtu)
dev->mtu = mtu;
}
- tunnel_dst_reset_all(t);
+ ip_tunnel_dst_reset_all(t);
netdev_state_change(dev);
}
@@ -1088,7 +1089,7 @@ void ip_tunnel_uninit(struct net_device *dev)
if (itn->fb_tunnel_dev != dev)
ip_tunnel_del(netdev_priv(dev));
- tunnel_dst_reset_all(tunnel);
+ ip_tunnel_dst_reset_all(tunnel);
}
EXPORT_SYMBOL_GPL(ip_tunnel_uninit);
diff --git a/net/ipv6/sit.c b/net/ipv6/sit.c
index 3dfbcf1dcb1c..b4d74c86586c 100644
--- a/net/ipv6/sit.c
+++ b/net/ipv6/sit.c
@@ -475,6 +475,7 @@ static void ipip6_tunnel_uninit(struct net_device *dev)
ipip6_tunnel_unlink(sitn, tunnel);
ipip6_tunnel_del_prl(tunnel, NULL);
}
+ ip_tunnel_dst_reset_all(tunnel);
dev_put(dev);
}
@@ -1082,6 +1083,7 @@ static void ipip6_tunnel_update(struct ip_tunnel *t, struct ip_tunnel_parm *p)
t->parms.link = p->link;
ipip6_tunnel_bind_dev(t->dev);
}
+ ip_tunnel_dst_reset_all(t);
netdev_state_change(t->dev);
}
@@ -1112,6 +1114,7 @@ static int ipip6_tunnel_update_6rd(struct ip_tunnel *t,
t->ip6rd.relay_prefix = relay_prefix;
t->ip6rd.prefixlen = ip6rd->prefixlen;
t->ip6rd.relay_prefixlen = ip6rd->relay_prefixlen;
+ ip_tunnel_dst_reset_all(t);
netdev_state_change(t->dev);
return 0;
}
@@ -1271,6 +1274,7 @@ ipip6_tunnel_ioctl (struct net_device *dev, struct ifreq *ifr, int cmd)
err = ipip6_tunnel_add_prl(t, &prl, cmd == SIOCCHGPRL);
break;
}
+ ip_tunnel_dst_reset_all(t);
netdev_state_change(dev);
break;
@@ -1326,6 +1330,9 @@ static const struct net_device_ops ipip6_netdev_ops = {
static void ipip6_dev_free(struct net_device *dev)
{
+ struct ip_tunnel *tunnel = netdev_priv(dev);
+
+ free_percpu(tunnel->dst_cache);
free_percpu(dev->tstats);
free_netdev(dev);
}
@@ -1375,6 +1382,12 @@ static int ipip6_tunnel_init(struct net_device *dev)
u64_stats_init(&ipip6_tunnel_stats->syncp);
}
+ tunnel->dst_cache = alloc_percpu(struct ip_tunnel_dst);
+ if (!tunnel->dst_cache) {
+ free_percpu(dev->tstats);
+ return -ENOMEM;
+ }
+
return 0;
}
@@ -1405,6 +1418,12 @@ static int __net_init ipip6_fb_tunnel_init(struct net_device *dev)
u64_stats_init(&ipip6_fb_stats->syncp);
}
+ tunnel->dst_cache = alloc_percpu(struct ip_tunnel_dst);
+ if (!tunnel->dst_cache) {
+ free_percpu(dev->tstats);
+ return -ENOMEM;
+ }
+
dev_hold(dev);
rcu_assign_pointer(sitn->tunnels_wc[0], tunnel);
return 0;
--
1.8.5.4
^ permalink raw reply related
* Re: [PATCH net-next v5 1/9] xen-netback: Introduce TX grant map definitions
From: Ian Campbell @ 2014-02-20 9:33 UTC (permalink / raw)
To: Zoltan Kiss; +Cc: wei.liu2, xen-devel, netdev, linux-kernel, jonathan.davies
In-Reply-To: <53050BF5.1060009@citrix.com>
On Wed, 2014-02-19 at 19:54 +0000, Zoltan Kiss wrote:
> On 19/02/14 10:05, Ian Campbell wrote:
> > On Tue, 2014-02-18 at 20:36 +0000, Zoltan Kiss wrote:
> >> On 18/02/14 17:06, Ian Campbell wrote:
> >>> On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> >>>> This patch contains the new definitions necessary for grant mapping.
> >>>
> >>> Is this just adding a bunch of (currently) unused functions? That's a
> >>> slightly odd way to structure a series. They don't seem to be "generic
> >>> helpers" or anything so it would be more normal to introduce these as
> >>> they get used -- it's a bit hard to review them out of context.
> >> I've created two patches because they are quite huge even now,
> >> separately. Together they would be a ~500 line change. That was the best
> >> I could figure out keeping in mind that bisect should work. But as I
> >> wrote in the first email, I welcome other suggestions. If you and Wei
> >> prefer this two patch in one big one, I merge them in the next version.
> >
> > I suppose it is hard to split a change like this up in a sensible way,
> > but it is rather hard to review something which is split in two parts
> > sensibly.
> >
> > If the combined patch too large to fit on the lists?
> Well, it's ca. 30 kb, ~500 lines changed. I guess it's possible. It's up
> to you and Wei, if you would like them to be merged, I can do that.
30kb doesn't sound too bad to me.
Patches #1 and #2 are, respectively:
drivers/net/xen-netback/common.h | 30 ++++++-
drivers/net/xen-netback/interface.c | 1 +
drivers/net/xen-netback/netback.c | 161 +++++++++++++++++++++++++++++++++++
3 files changed, 191 insertions(+), 1 deletion(-)
drivers/net/xen-netback/interface.c | 63 ++++++++-
drivers/net/xen-netback/netback.c | 254 ++++++++++++++---------------------
2 files changed, 160 insertions(+), 157 deletions(-)
I don't think combining those would be terrible, although I'm willing to
be proven wrong ;-)
> >>>
> >>>> + vif->dealloc_prod++;
> >>>
> >>> What happens if the dealloc ring becomes full, will this wrap and cause
> >>> havoc?
> >> Nope, if the dealloc ring is full, the value of the last increment won't
> >> be used to index the dealloc ring again until some space made available.
> >
> > I don't follow -- what makes this the case?
> The dealloc ring has the same size as the pending ring, and you can only
> add slots to it which are already on the pending ring (the pending_idx
> comes from ubuf->desc), as you are essentially free up slots here on the
> pending ring.
> So if the dealloc ring becomes full, vif->dealloc_prod -
> vif->dealloc_cons will be 256, which would be bad. But the while loop
> should exit here, as we shouldn't have any more pending slots. And if we
> dealloc and create free pending slots in dealloc_action, dealloc_cons
> will also advance.
OK, so this is limited by the size of the pending array, makes sense,
assuming that array is itself correctly guarded...
> >> Of course if something broke and we have more pending slots than tx ring
> >> or dealloc slots then it can happen. Do you suggest a
> >> BUG_ON(vif->dealloc_prod - vif->dealloc_cons >= MAX_PENDING_REQS)?
> >
> > A
> > BUG_ON(space in dealloc ring < number of slots needed to dealloc this skb)
> > would seem to be the right thing, if that really is the invariant the
> > code is supposed to be implementing.
> Not exactly, it means BUG_ON(number of slots to dealloc >
> MAX_PENDING_REQS), and it should be at the end of the loop, without '='.
OK.
> >
> >>>
> >>>> + }
> >>>> +
> >>>> + } while (dp != vif->dealloc_prod);
> >>>> +
> >>>> + vif->dealloc_cons = dc;
> >>>
> >>> No barrier here?
> >> dealloc_cons only used in the dealloc_thread. dealloc_prod is used by
> >> the callback and the thread as well, that's why we need mb() in
> >> previous. Btw. this function comes from classic's net_tx_action_dealloc
> >
> > Is this code close enough to that code architecturally that you can
> > infer correctness due to that though?
> Nope, I've just mentioned it because knowing that old code can help to
> understand this new, as their logic is very similar some places, like here.
>
> > So long as you have considered the barrier semantics in the context of
> > the current code and you think it is correct to not have one here then
> > I'm ok. But if you have just assumed it is OK because some older code
> > didn't have it then I'll have to ask you to consider it again...
> Nope, as I mentioned above, dealloc_cons only accessed in that funcion,
> from the same thread. Dealloc_prod is written in the callback and read
> out here, that's why we need the barrier there.
OK.
Although this may no longer be true if you added some BUG_ONs as
discussed above?
>
> >
> >>>> + netdev_err(vif->dev,
> >>>> + " host_addr: %llx handle: %x status: %d\n",
> >>>> + gop[i].host_addr,
> >>>> + gop[i].handle,
> >>>> + gop[i].status);
> >>>> + }
> >>>> + BUG();
> >>>> + }
> >>>> + }
> >>>> +
> >>>> + for (i = 0; i < gop - vif->tx_unmap_ops; ++i)
> >>>> + xenvif_idx_release(vif, pending_idx_release[i],
> >>>> + XEN_NETIF_RSP_OKAY);
> >>>> +}
> >>>> +
> >>>> +
> >>>> /* Called after netfront has transmitted */
> >>>> int xenvif_tx_action(struct xenvif *vif, int budget)
> >>>> {
> >>>> @@ -1678,6 +1793,25 @@ static void xenvif_idx_release(struct xenvif *vif, u16 pending_idx,
> >>>> vif->mmap_pages[pending_idx] = NULL;
> >>>> }
> >>>>
> >>>> +void xenvif_idx_unmap(struct xenvif *vif, u16 pending_idx)
> >>>
> >>> This is a single shot version of the batched xenvif_tx_dealloc_action
> >>> version? Why not just enqueue the idx to be unmapped later?
> >> This is called only from the NAPI instance. Using the dealloc ring
> >> require synchronization with the callback which can increase lock
> >> contention. On the other hand, if the guest sends small packets
> >> (<PAGE_SIZE), the TLB flushing can cause performance penalty.
> >
> > Right. When/How often is this called from the NAPI instance?
> When grant mapping error detected in xenvif_tx_check_gop, and if a
> packet smaller than PKT_PROT_LEN is sent. The latter would be removed if
> we will grant copy such packets entirely.
>
> > Is the locking contention from this case so severe that it out weighs
> > the benefits of batching the unmaps? That would surprise me. After all
> > the locking contention is there for the zerocopy_callback case too
> >
> >> The above
> >> mentioned upcoming patch which gntcopy the header can prevent that
> >
> > So this is only called when doing the pull-up to the linear area?
> Yes, as mentioned above.
I'm not sure why you don't just enqueue the dealloc with the other
normal ones though.
Ian.
^ permalink raw reply
* RE: [PATCH] be2net: Bugfix for packet drop with kernel param swiotlb=force
From: Sathya Perla @ 2014-02-20 9:39 UTC (permalink / raw)
To: jiang.biao2@zte.com.cn, netdev@vger.kernel.org
Cc: Subramanian Seetharaman, Ajit Khaparde, wang.liang82@zte.com.cn,
cai.qu@zte.com.cn, li.fengmao@zte.com.cn, long.chun@zte.com.cn,
David Miller
In-Reply-To: <OF1C38716E.D9F528E0-ON48257C85.000A026B-48257C85.000AA8C9@zte.com.cn>
> -----Original Message-----
> From: jiang.biao2@zte.com.cn [mailto:jiang.biao2@zte.com.cn]
>
>
> From: Li Fengmao <li.fengmao@zte.com.cn>
>
> There will be packet drop with kernel param "swiotlb = force" on
> Emulex 10Gb NIC using be2net driver. The problem is caused by
> receiving skb without calling pci_unmap_page() in get_rx_page_info().
> rx_page_info->last_page_user is initialized to false in
> be_post_rx_frags() when current frag are mapped in the first half of
> the same page with another frag. But in that case with
> "swiotlb = force" param, data can not be copied into the page of
> rx_page_info without calling pci_unmap_page, so the data frag mapped
> in the first half of the page will be dropped.
>
> It can be solved by creating only a mapping relation between frag
> and page, and deleting rx_page_info->last_page_user to ensure
> calling pci_unmap_page when handling each receiving frag.
This patch uses an entire page for each RX frag (whose default size is 2048).
Consequently, on platforms like ppc64 where the default PAGE_SIZE is 64K,
memory usage becomes very inefficient.
Instead, I've tried a partial-page mapping scheme. This retains the
page sharing logic, but un-maps each frag separately so that
the data is copied from the bounce buffers.
Pls see if this works for you; thanks.
---
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index a150401..013777a 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -263,7 +263,6 @@ struct be_rx_page_info {
struct page *page;
DEFINE_DMA_UNMAP_ADDR(bus);
u16 page_offset;
- bool last_page_user;
};
struct be_rx_stats {
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 4f87f5c..9d8ea91 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1448,13 +1448,8 @@ static struct be_rx_page_info *get_rx_page_info(struct be_rx_obj *rxo)
rx_page_info = &rxo->page_info_tbl[frag_idx];
BUG_ON(!rx_page_info->page);
- if (rx_page_info->last_page_user) {
- dma_unmap_page(&adapter->pdev->dev,
- dma_unmap_addr(rx_page_info, bus),
- adapter->big_page_size, DMA_FROM_DEVICE);
- rx_page_info->last_page_user = false;
- }
-
+ dma_unmap_page(&adapter->pdev->dev, dma_unmap_addr(rx_page_info, bus),
+ rx_frag_size, DMA_FROM_DEVICE);
queue_tail_inc(rxq);
atomic_dec(&rxq->used);
return rx_page_info;
@@ -1761,12 +1756,12 @@ static inline struct page *be_alloc_pages(u32 size, gfp_t gfp)
static void be_post_rx_frags(struct be_rx_obj *rxo, gfp_t gfp)
{
struct be_adapter *adapter = rxo->adapter;
- struct be_rx_page_info *page_info = NULL, *prev_page_info = NULL;
+ struct be_rx_page_info *page_info = NULL;
struct be_queue_info *rxq = &rxo->q;
struct page *pagep = NULL;
struct device *dev = &adapter->pdev->dev;
struct be_eth_rx_d *rxd;
- u64 page_dmaaddr = 0, frag_dmaaddr;
+ u64 frag_dmaaddr;
u32 posted, page_offset = 0;
page_info = &rxo->page_info_tbl[rxq->head];
@@ -1777,24 +1772,22 @@ static void be_post_rx_frags(struct be_rx_obj *rxo, gfp_t gfp)
rx_stats(rxo)->rx_post_fail++;
break;
}
- page_dmaaddr = dma_map_page(dev, pagep, 0,
- adapter->big_page_size,
- DMA_FROM_DEVICE);
- if (dma_mapping_error(dev, page_dmaaddr)) {
- put_page(pagep);
- pagep = NULL;
- rx_stats(rxo)->rx_post_fail++;
- break;
- }
page_info->page_offset = 0;
} else {
get_page(pagep);
page_info->page_offset = page_offset + rx_frag_size;
}
page_offset = page_info->page_offset;
+
+ frag_dmaaddr = dma_map_page(dev, pagep, page_offset,
+ rx_frag_size, DMA_FROM_DEVICE);
+ if (dma_mapping_error(dev, frag_dmaaddr)) {
+ put_page(pagep);
+ rx_stats(rxo)->rx_post_fail++;
+ break;
+ }
page_info->page = pagep;
- dma_unmap_addr_set(page_info, bus, page_dmaaddr);
- frag_dmaaddr = page_dmaaddr + page_info->page_offset;
+ dma_unmap_addr_set(page_info, bus, frag_dmaaddr);
rxd = queue_head_node(rxq);
rxd->fragpa_lo = cpu_to_le32(frag_dmaaddr & 0xFFFFFFFF);
@@ -1802,17 +1795,12 @@ static void be_post_rx_frags(struct be_rx_obj *rxo, gfp_t gfp)
/* Any space left in the current big page for another frag? */
if ((page_offset + rx_frag_size + rx_frag_size) >
- adapter->big_page_size) {
+ adapter->big_page_size)
pagep = NULL;
- page_info->last_page_user = true;
- }
- prev_page_info = page_info;
queue_head_inc(rxq);
page_info = &rxo->page_info_tbl[rxq->head];
}
- if (pagep)
- prev_page_info->last_page_user = true;
if (posted) {
atomic_add(posted, &rxq->used);
>
> Steps to reproduce the bug:
> 1. Prepare a Emulex Corporation OneConnect 10Gb NIC.
> 2. Add the kernel param like "swiotlb = force" in /boot/grub/grub.conf .
> 3. Reboot the system. (e.g exec reboot command)
> 4. Activate the interface. (e.g ifconfig eth0 192.168.1.2 up)
> 5. There will be packet drop when ping 192.168.1.2 from another host.
>
> Signed-off-by: Li Fengmao <li.fengmao@zte.com.cn>
> Signed-off-by: Long Chun <long.chun@zte.com.cn>
> Reviewed-by: Wang Liang <wang.liang82@zte.com.cn>
> Reviewed-by: Cai Qu <cai.qu@zte.com.cn>
> Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>
>
> --- old/drivers/net/ethernet/emulex/benet/be_main.c 2014-02-20
> 08:49:49.322503588 +0800
> +++ new/drivers/net/ethernet/emulex/benet/be_main.c 2014-02-20
> 08:56:38.796503104 +0800
> @@ -1018,12 +1018,9 @@ get_rx_page_info(struct be_adapter *adap
> rx_page_info = &rxo->page_info_tbl[frag_idx];
> BUG_ON(!rx_page_info->page);
>
> - if (rx_page_info->last_page_user) {
> - dma_unmap_page(&adapter->pdev->dev,
> - dma_unmap_addr(rx_page_info, bus),
> - adapter->big_page_size, DMA_FROM_DEVICE);
> - rx_page_info->last_page_user = false;
> - }
> + dma_unmap_page(&adapter->pdev->dev,
> + dma_unmap_addr(rx_page_info, bus),
> + rx_frag_size, DMA_FROM_DEVICE);
>
> atomic_dec(&rxq->used);
> return rx_page_info;
> @@ -1344,20 +1341,15 @@ static void be_post_rx_frags(struct be_r
>
> page_info = &rxo->page_info_tbl[rxq->head];
> for (posted = 0; posted < MAX_RX_POST && !page_info->page; posted++) {
> - if (!pagep) {
> - pagep = be_alloc_pages(adapter->big_page_size, gfp);
> - if (unlikely(!pagep)) {
> - rx_stats(rxo)->rx_post_fail++;
> - break;
> - }
> - page_dmaaddr = dma_map_page(&adapter->pdev->dev, pagep,
> - 0, adapter->big_page_size,
> - DMA_FROM_DEVICE);
> - page_info->page_offset = 0;
> - } else {
> - get_page(pagep);
> - page_info->page_offset = page_offset + rx_frag_size;
> + pagep = be_alloc_pages(rx_frag_size, gfp);
> + if (unlikely(!pagep)) {
> + rx_stats(rxo)->rx_post_fail++;
> + break;
> }
> + page_dmaaddr = dma_map_page(&adapter->pdev->dev, pagep,
> + 0, rx_frag_size,
> + DMA_FROM_DEVICE);
> + page_info->page_offset = 0;
> page_offset = page_info->page_offset;
> page_info->page = pagep;
> dma_unmap_addr_set(page_info, bus, page_dmaaddr);
> @@ -1367,12 +1359,7 @@ static void be_post_rx_frags(struct be_r
> rxd->fragpa_lo = cpu_to_le32(frag_dmaaddr & 0xFFFFFFFF);
> rxd->fragpa_hi = cpu_to_le32(upper_32_bits(frag_dmaaddr));
>
> - /* Any space left in the current big page for another frag? */
> - if ((page_offset + rx_frag_size + rx_frag_size) >
> - adapter->big_page_size) {
> - pagep = NULL;
> - page_info->last_page_user = true;
> - }
> + pagep = NULL;
>
> prev_page_info = page_info;
> queue_head_inc(rxq);
^ permalink raw reply related
* Re: [PATCH net-next] xfrm: Correctly parse netlink msg from 32bits ip command on 64bits host
From: Steffen Klassert @ 2014-02-20 9:59 UTC (permalink / raw)
To: Fan Du; +Cc: davem, netdev
In-Reply-To: <1392801176-2656-1-git-send-email-fan.du@windriver.com>
On Wed, Feb 19, 2014 at 05:12:56PM +0800, Fan Du wrote:
>
> +#ifdef CONFIG_COMPAT
> +
> +struct compat_xfrm_userpolicy_info {
> + struct xfrm_selector sel;
> + struct xfrm_lifetime_cfg lft;
> + struct xfrm_lifetime_cur curlft;
> + __u32 priority;
> + __u32 index;
> + __u8 dir;
> + __u8 action;
> +#define XFRM_POLICY_ALLOW 0
> +#define XFRM_POLICY_BLOCK 1
> + __u8 flags;
> +#define XFRM_POLICY_LOCALOK 1 /* Allow user to override global policy */
> + /* Automatically expand selector to include matching ICMP payloads. */
> +#define XFRM_POLICY_ICMP 2
> + __u8 share;
> +} __attribute__((packed));
> +
> +struct compat_xfrm_usersa_info {
> + struct xfrm_selector sel;
> + struct xfrm_id id;
> + xfrm_address_t saddr;
> + struct xfrm_lifetime_cfg lft;
> + struct xfrm_lifetime_cur curlft;
> + struct xfrm_stats stats;
> + __u32 seq;
> + __u32 reqid;
> + __u16 family;
> + __u8 mode; /* XFRM_MODE_xxx */
> + __u8 replay_window;
> + __u8 flags;
> +#define XFRM_STATE_NOECN 1
> +#define XFRM_STATE_DECAP_DSCP 2
> +#define XFRM_STATE_NOPMTUDISC 4
> +#define XFRM_STATE_WILDRECV 8
> +#define XFRM_STATE_ICMP 16
> +#define XFRM_STATE_AF_UNSPEC 32
> +#define XFRM_STATE_ALIGN4 64
> +#define XFRM_STATE_ESN 128
> +} __attribute__((packed));
> +
You define two structures that you actually don't use,
all you do is checking their size.
The only reason why this works is because these two
structures differ only on some padding bytes at the
end. If we want to support 32 bit ipsec tools on
64 bit kernels, we need a complete compat layer
for all the userspace exported structures.
A lot of userspace exported structures differ not only
on some padding bytes at the end.
For example the layout of xfrm_userspi_info:
on 32 bit:
struct xfrm_userspi_info {
struct xfrm_usersa_info info; /* 0 220 */
/* XXX last struct has 3 bytes of padding */
/* --- cacheline 3 boundary (192 bytes) was 28 bytes ago --- */
__u32 min; /* 220 4 */
__u32 max; /* 224 4 */
/* size: 228, cachelines: 4, members: 3 */
/* paddings: 1, sum paddings: 3 */
/* last cacheline: 36 bytes */
};
on 64 bit:
struct xfrm_userspi_info {
struct xfrm_usersa_info info; /* 0 224 */
/* XXX last struct has 7 bytes of padding */
/* --- cacheline 3 boundary (192 bytes) was 32 bytes ago --- */
__u32 min; /* 224 4 */
__u32 max; /* 228 4 */
/* size: 232, cachelines: 4, members: 3 */
/* paddings: 1, sum paddings: 7 */
/* last cacheline: 40 bytes */
};
So the 'min' field has an offest of 220 bytes on 32 bit and an offset
of 224 bytes on 64 bit. We would need a compatability layer like we
have it for system calls to map this correct.
For now I think we should just refuse to do anything if someone tries
to configure ipsec with 32 bit tools on a 64 bit machine.
^ permalink raw reply
* Re: [PATCH net-next v5 1/9] xen-netback: Introduce TX grant map definitions
From: Wei Liu @ 2014-02-20 10:13 UTC (permalink / raw)
To: Zoltan Kiss
Cc: Ian Campbell, wei.liu2, xen-devel, netdev, linux-kernel,
jonathan.davies
In-Reply-To: <53050BF5.1060009@citrix.com>
On Wed, Feb 19, 2014 at 07:54:29PM +0000, Zoltan Kiss wrote:
> On 19/02/14 10:05, Ian Campbell wrote:
> >On Tue, 2014-02-18 at 20:36 +0000, Zoltan Kiss wrote:
> >>On 18/02/14 17:06, Ian Campbell wrote:
> >>>On Mon, 2014-01-20 at 21:24 +0000, Zoltan Kiss wrote:
> >>>>This patch contains the new definitions necessary for grant mapping.
> >>>
> >>>Is this just adding a bunch of (currently) unused functions? That's a
> >>>slightly odd way to structure a series. They don't seem to be "generic
> >>>helpers" or anything so it would be more normal to introduce these as
> >>>they get used -- it's a bit hard to review them out of context.
> >>I've created two patches because they are quite huge even now,
> >>separately. Together they would be a ~500 line change. That was the best
> >>I could figure out keeping in mind that bisect should work. But as I
> >>wrote in the first email, I welcome other suggestions. If you and Wei
> >>prefer this two patch in one big one, I merge them in the next version.
> >
> >I suppose it is hard to split a change like this up in a sensible way,
> >but it is rather hard to review something which is split in two parts
> >sensibly.
> >
> >If the combined patch too large to fit on the lists?
> Well, it's ca. 30 kb, ~500 lines changed. I guess it's possible.
> It's up to you and Wei, if you would like them to be merged, I can
> do that.
>
As I said before, my bottom line is "don't break bisection". Do whatever
you want to. :-)
Wei.
^ permalink raw reply
* [PATCH] net: fec: fix potential issue to avoid fec interrupt lost and crc error
From: Fugang Duan @ 2014-02-20 10:14 UTC (permalink / raw)
To: b20596, davem; +Cc: netdev
The current flow: Set TX BD ready, and then set "INT" and "PINS" bit to
enable tx interrupt generation and crc checksum.
There has potential issue like as:
CPU fec uDMA
Set tx ready bit
uDMA start the BD transmission
Set "INT" bit
Set "PINS" bit
...
Above situation cause fec tx interrupt lost and fec MAC don't do
CRC checksum. The patch fix the potential issue.
Signed-off-by: Fugang Duan <B38611@freescale.com>
---
drivers/net/ethernet/freescale/fec_main.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 45b8b22..739608b 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -390,12 +390,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
netdev_err(ndev, "Tx DMA memory map failed\n");
return NETDEV_TX_OK;
}
- /* Send it on its way. Tell FEC it's ready, interrupt when done,
- * it's the last BD of the frame, and to put the CRC on the end.
- */
- status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR
- | BD_ENET_TX_LAST | BD_ENET_TX_TC);
- bdp->cbd_sc = status;
if (fep->bufdesc_ex) {
@@ -417,6 +411,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
}
}
+ /* Send it on its way. Tell FEC it's ready, interrupt when done,
+ * it's the last BD of the frame, and to put the CRC on the end.
+ */
+ status |= (BD_ENET_TX_READY | BD_ENET_TX_INTR
+ | BD_ENET_TX_LAST | BD_ENET_TX_TC);
+ bdp->cbd_sc = status;
+
bdp_pre = fec_enet_get_prevdesc(bdp, fep);
if ((id_entry->driver_data & FEC_QUIRK_ERR006358) &&
!(bdp_pre->cbd_sc & BD_ENET_TX_READY)) {
--
1.7.2.rc3
^ permalink raw reply related
* [PATCH net] bonding: fix bond_arp_rcv() race of curr_active_slave
From: Veaceslav Falico @ 2014-02-20 11:07 UTC (permalink / raw)
To: netdev; +Cc: dingtianhong, Veaceslav Falico, Jay Vosburgh, Andy Gospodarek
bond->curr_active_slave can be changed between its deferences, even to
NULL, and thus we might panic.
We're always holding the rcu (rx_handler->bond_handle_frame()->bond_arp_rcv())
so fix this by rcu_dereferencing() it and using the saved.
Reported-by: Ding Tianhong <dingtianhong@huawei.com>
Fixes: aeea64a ("bonding: don't trust arp requests unless active slave really works")
CC: Jay Vosburgh <fubar@us.ibm.com>
CC: Andy Gospodarek <andy@greyhouse.net>
Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
---
drivers/net/bonding/bond_main.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 71edf03..bd70bbc 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -2254,6 +2254,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
struct slave *slave)
{
struct arphdr *arp = (struct arphdr *)skb->data;
+ struct slave *curr_active_slave;
unsigned char *arp_ptr;
__be32 sip, tip;
int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
@@ -2299,6 +2300,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
bond->params.arp_validate, slave_do_arp_validate(bond, slave),
&sip, &tip);
+ curr_active_slave = rcu_dereference(bond->curr_active_slave);
+
/*
* Backup slaves won't see the ARP reply, but do come through
* here for each ARP probe (so we swap the sip/tip to validate
@@ -2312,11 +2315,12 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
* is done to avoid endless looping when we can't reach the
* arp_ip_target and fool ourselves with our own arp requests.
*/
+
if (bond_is_active_slave(slave))
bond_validate_arp(bond, slave, sip, tip);
- else if (bond->curr_active_slave &&
- time_after(slave_last_rx(bond, bond->curr_active_slave),
- bond->curr_active_slave->last_link_up))
+ else if (curr_active_slave &&
+ time_after(slave_last_rx(bond, curr_active_slave),
+ curr_active_slave->last_link_up))
bond_validate_arp(bond, slave, tip, sip);
out_unlock:
--
1.8.4
^ permalink raw reply related
* Re: [PATCH net] bonding: fix bond_arp_rcv() race of curr_active_slave
From: Veaceslav Falico @ 2014-02-20 11:15 UTC (permalink / raw)
To: netdev; +Cc: dingtianhong, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392894477-5477-1-git-send-email-vfalico@redhat.com>
On Thu, Feb 20, 2014 at 12:07:57PM +0100, Veaceslav Falico wrote:
>bond->curr_active_slave can be changed between its deferences, even to
>NULL, and thus we might panic.
>
>We're always holding the rcu (rx_handler->bond_handle_frame()->bond_arp_rcv())
>so fix this by rcu_dereferencing() it and using the saved.
>
>Reported-by: Ding Tianhong <dingtianhong@huawei.com>
>Fixes: aeea64a ("bonding: don't trust arp requests unless active slave really works")
>CC: Jay Vosburgh <fubar@us.ibm.com>
>CC: Andy Gospodarek <andy@greyhouse.net>
>Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
Sorry David, it should have been net-next (I've based it on the net-next
tree).
Can you apply it to net-next or should I resend it?
Sorry for the noise.
>---
> drivers/net/bonding/bond_main.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 71edf03..bd70bbc 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2254,6 +2254,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> struct slave *slave)
> {
> struct arphdr *arp = (struct arphdr *)skb->data;
>+ struct slave *curr_active_slave;
> unsigned char *arp_ptr;
> __be32 sip, tip;
> int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
>@@ -2299,6 +2300,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> bond->params.arp_validate, slave_do_arp_validate(bond, slave),
> &sip, &tip);
>
>+ curr_active_slave = rcu_dereference(bond->curr_active_slave);
>+
> /*
> * Backup slaves won't see the ARP reply, but do come through
> * here for each ARP probe (so we swap the sip/tip to validate
>@@ -2312,11 +2315,12 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> * is done to avoid endless looping when we can't reach the
> * arp_ip_target and fool ourselves with our own arp requests.
> */
>+
> if (bond_is_active_slave(slave))
> bond_validate_arp(bond, slave, sip, tip);
>- else if (bond->curr_active_slave &&
>- time_after(slave_last_rx(bond, bond->curr_active_slave),
>- bond->curr_active_slave->last_link_up))
>+ else if (curr_active_slave &&
>+ time_after(slave_last_rx(bond, curr_active_slave),
>+ curr_active_slave->last_link_up))
> bond_validate_arp(bond, slave, tip, sip);
>
> out_unlock:
>--
>1.8.4
>
^ permalink raw reply
* Re: [PATCH net] bonding: fix bond_arp_rcv() race of curr_active_slave
From: Ding Tianhong @ 2014-02-20 11:49 UTC (permalink / raw)
To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1392894477-5477-1-git-send-email-vfalico@redhat.com>
On 2014/2/20 19:07, Veaceslav Falico wrote:
> bond->curr_active_slave can be changed between its deferences, even to
> NULL, and thus we might panic.
>
> We're always holding the rcu (rx_handler->bond_handle_frame()->bond_arp_rcv())
> so fix this by rcu_dereferencing() it and using the saved.
>
> Reported-by: Ding Tianhong <dingtianhong@huawei.com>
> Fixes: aeea64a ("bonding: don't trust arp requests unless active slave really works")
> CC: Jay Vosburgh <fubar@us.ibm.com>
> CC: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Veaceslav Falico <vfalico@redhat.com>
> ---
> drivers/net/bonding/bond_main.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 71edf03..bd70bbc 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2254,6 +2254,7 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> struct slave *slave)
> {
> struct arphdr *arp = (struct arphdr *)skb->data;
> + struct slave *curr_active_slave;
> unsigned char *arp_ptr;
> __be32 sip, tip;
> int alen, is_arp = skb->protocol == __cpu_to_be16(ETH_P_ARP);
> @@ -2299,6 +2300,8 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> bond->params.arp_validate, slave_do_arp_validate(bond, slave),
> &sip, &tip);
>
> + curr_active_slave = rcu_dereference(bond->curr_active_slave);
> +
> /*
> * Backup slaves won't see the ARP reply, but do come through
> * here for each ARP probe (so we swap the sip/tip to validate
> @@ -2312,11 +2315,12 @@ int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond,
> * is done to avoid endless looping when we can't reach the
> * arp_ip_target and fool ourselves with our own arp requests.
> */
> +
> if (bond_is_active_slave(slave))
> bond_validate_arp(bond, slave, sip, tip);
> - else if (bond->curr_active_slave &&
> - time_after(slave_last_rx(bond, bond->curr_active_slave),
> - bond->curr_active_slave->last_link_up))
> + else if (curr_active_slave &&
> + time_after(slave_last_rx(bond, curr_active_slave),
> + curr_active_slave->last_link_up))
> bond_validate_arp(bond, slave, tip, sip);
>
> out_unlock:
>
Acked-by: Ding Tianhong <dingtianhong@huawei.com>
^ permalink raw reply
* [PATCH net] net: sctp: fix multihoming retransmission path selection to rfc4960
From: Daniel Borkmann @ 2014-02-20 11:53 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-sctp, Gui Jianfeng
Problem statement: 1) both paths (primary path1 and alternate
path2) are up after the association has been established i.e.,
HB packets are normally exchanged, 2) path2 gets inactive after
path_max_retrans * max_rto timed out (i.e. path2 is down completely),
3) now, if a transmission times out on the only surviving/active
path1 (any ~1sec network service impact could cause this like
a channel bonding failover), then the retransmitted packets are
sent over the inactive path2; this happens with partial failover
and without it.
Besides not being optimal in the above scenario, a small failure
or timeout in the only existing path has the potential to cause
long delays in the retransmission (depending on RTO_MAX) until
the still active path is reselected.
RFC4960, section 6.4. "Multi-Homed SCTP Endpoints" states under
6.4.1. "Failover from an Inactive Destination Address" the
following:
Some of the transport addresses of a multi-homed SCTP endpoint
may become inactive due to either the occurrence of certain
error conditions (see Section 8.2) or adjustments from the
SCTP user.
When there is outbound data to send and the primary path
becomes inactive (e.g., due to failures), or where the SCTP
user explicitly requests to send data to an inactive
destination transport address, before reporting an error to
its ULP, the SCTP endpoint should try to send the data to an
alternate *active* destination transport address if one exists.
When retransmitting data that timed out, if the endpoint is
multihomed, it should consider each source-destination address
pair in its retransmission selection policy. When retransmitting
timed-out data, the endpoint should attempt to pick the most
divergent source-destination pair from the original
source-destination pair to which the packet was transmitted.
Note: Rules for picking the most divergent source-destination
pair are an implementation decision and are not specified
within this document.
So, we should first reconsider to take the current active
retransmission transport if we cannot find an alternative
active one, as otherwise, by sending a user message to an
inactive destination transport address while excluding an
active destination transport address, we would not comply
to RFC4960. If all of that fails, we can still round robin
through unkown, partial failover, and inactive ones in the
hope to find something still suitable/useful.
Commit 4141ddc02a92 ("sctp: retran_path update bug fix") broke
that behaviour by selecting the next non-active transport when
no other active transport was found besides the current assoc's
peer.retran_path. Before commit 4141ddc02a92, we would have
traversed through the list until we reach our peer.retran_path
again, and in case that is still in state SCTP_ACTIVE, we would
take it and return. Only if that is not the case either, we
take the next inactive transport. Besides all that, another
issue is that transports in state SCTP_UNKNOWN could be preferred
over transports in state SCTP_ACTIVE in case a SCTP_ACTIVE
transport appears after SCTP_UNKNOWN in the transport list
yielding a "weaker" transport state to be used in retransmission.
This patch mostly reverts 4141ddc02a92, but also rewrites
this function to introduce more clarity and strictness into
the code. A strict priority of transport states is enforced
in this patch, hence selection is active > unkown > partial
failover > inactive.
Fixes: 4141ddc02a92 ("sctp: retran_path update bug fix")
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
Cc: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
---
net/sctp/associola.c | 123 ++++++++++++++++++++++++++++++---------------------
1 file changed, 73 insertions(+), 50 deletions(-)
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index f558433..bac47a4 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1239,78 +1239,101 @@ void sctp_assoc_update(struct sctp_association *asoc,
}
/* Update the retran path for sending a retransmitted packet.
- * Round-robin through the active transports, else round-robin
- * through the inactive transports as this is the next best thing
- * we can try.
+ * See also RFC4960, 6.4. Multi-Homed SCTP Endpoints:
+ *
+ * When there is outbound data to send and the primary path
+ * becomes inactive (e.g., due to failures), or where the
+ * SCTP user explicitly requests to send data to an
+ * inactive destination transport address, before reporting
+ * an error to its ULP, the SCTP endpoint should try to send
+ * the data to an alternate active destination transport
+ * address if one exists.
+ *
+ * When retransmitting data that timed out, if the endpoint
+ * is multihomed, it should consider each source-destination
+ * address pair in its retransmission selection policy.
+ * When retransmitting timed-out data, the endpoint should
+ * attempt to pick the most divergent source-destination
+ * pair from the original source-destination pair to which
+ * the packet was transmitted.
+ *
+ * Note: Rules for picking the most divergent source-destination
+ * pair are an implementation decision and are not specified
+ * within this document.
+ *
+ * Our basic strategy is to round-robin transports in priorities
+ * according to sctp_state_prio_map[] e.g., if no such
+ * transport with state SCTP_ACTIVE exists, round-robin through
+ * SCTP_UNKNOWN, etc. You get the picture.
*/
-void sctp_assoc_update_retran_path(struct sctp_association *asoc)
+static const u8 sctp_trans_state_to_prio_map[] = {
+ [SCTP_ACTIVE] = 3, /* best case */
+ [SCTP_UNKNOWN] = 2,
+ [SCTP_PF] = 1,
+ [SCTP_INACTIVE] = 0, /* worst case */
+};
+
+static u8 sctp_trans_score(const struct sctp_transport *trans)
{
- struct sctp_transport *t, *next;
- struct list_head *head = &asoc->peer.transport_addr_list;
- struct list_head *pos;
+ return sctp_trans_state_to_prio_map[trans->state];
+}
- if (asoc->peer.transport_count == 1)
- return;
+static struct sctp_transport *sctp_trans_elect_best(struct sctp_transport *curr,
+ struct sctp_transport *best)
+{
+ if (best == NULL)
+ return curr;
- /* Find the next transport in a round-robin fashion. */
- t = asoc->peer.retran_path;
- pos = &t->transports;
- next = NULL;
+ return sctp_trans_score(curr) > sctp_trans_score(best) ? curr : best;
+}
- while (1) {
- /* Skip the head. */
- if (pos->next == head)
- pos = head->next;
- else
- pos = pos->next;
+void sctp_assoc_update_retran_path(struct sctp_association *asoc)
+{
+ struct sctp_transport *trans = asoc->peer.retran_path;
+ struct sctp_transport *trans_next = NULL;
- t = list_entry(pos, struct sctp_transport, transports);
+ /* We're done as we only have the one and only path. */
+ if (asoc->peer.transport_count == 1)
+ return;
- /* We have exhausted the list, but didn't find any
- * other active transports. If so, use the next
- * transport.
- */
- if (t == asoc->peer.retran_path) {
- t = next;
+ /* Iterate from retran_path's successor back to retran_path. */
+ for (trans = list_next_entry(trans, transports); 1;
+ trans = list_next_entry(trans, transports)) {
+ /* Manually skip the head element. */
+ if (&trans->transports == &asoc->peer.transport_addr_list)
+ continue;
+ if (trans->state == SCTP_UNCONFIRMED)
+ continue;
+ trans_next = sctp_trans_elect_best(trans, trans_next);
+ /* Active is good enough for immediate return. */
+ if (trans_next->state == SCTP_ACTIVE)
break;
- }
-
- /* Try to find an active transport. */
-
- if ((t->state == SCTP_ACTIVE) ||
- (t->state == SCTP_UNKNOWN)) {
+ /* We've reached the end, time to update path. */
+ if (trans == asoc->peer.retran_path)
break;
- } else {
- /* Keep track of the next transport in case
- * we don't find any active transport.
- */
- if (t->state != SCTP_UNCONFIRMED && !next)
- next = t;
- }
}
- if (t)
- asoc->peer.retran_path = t;
- else
- t = asoc->peer.retran_path;
+ if (trans_next != NULL)
+ asoc->peer.retran_path = trans_next;
- pr_debug("%s: association:%p addr:%pISpc\n", __func__, asoc,
- &t->ipaddr.sa);
+ pr_debug("%s: association:%p updated new path to addr:%pISpc\n",
+ __func__, asoc, &asoc->peer.retran_path->ipaddr.sa);
}
-/* Choose the transport for sending retransmit packet. */
-struct sctp_transport *sctp_assoc_choose_alter_transport(
- struct sctp_association *asoc, struct sctp_transport *last_sent_to)
+struct sctp_transport *
+sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
+ struct sctp_transport *last_sent_to)
{
/* If this is the first time packet is sent, use the active path,
* else use the retran path. If the last packet was sent over the
* retran path, update the retran path and use it.
*/
- if (!last_sent_to)
+ if (last_sent_to == NULL) {
return asoc->peer.active_path;
- else {
+ } else {
if (last_sent_to == asoc->peer.retran_path)
sctp_assoc_update_retran_path(asoc);
+
return asoc->peer.retran_path;
}
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH net] net: sctp: fix multihoming retransmission path selection to rfc4960
From: Neil Horman @ 2014-02-20 12:01 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: davem, netdev, linux-sctp, Gui Jianfeng
In-Reply-To: <1392897186-26841-1-git-send-email-dborkman@redhat.com>
On Thu, Feb 20, 2014 at 12:53:06PM +0100, Daniel Borkmann wrote:
> Problem statement: 1) both paths (primary path1 and alternate
> path2) are up after the association has been established i.e.,
> HB packets are normally exchanged, 2) path2 gets inactive after
> path_max_retrans * max_rto timed out (i.e. path2 is down completely),
> 3) now, if a transmission times out on the only surviving/active
> path1 (any ~1sec network service impact could cause this like
> a channel bonding failover), then the retransmitted packets are
> sent over the inactive path2; this happens with partial failover
> and without it.
>
> Besides not being optimal in the above scenario, a small failure
> or timeout in the only existing path has the potential to cause
> long delays in the retransmission (depending on RTO_MAX) until
> the still active path is reselected.
>
> RFC4960, section 6.4. "Multi-Homed SCTP Endpoints" states under
> 6.4.1. "Failover from an Inactive Destination Address" the
> following:
>
> Some of the transport addresses of a multi-homed SCTP endpoint
> may become inactive due to either the occurrence of certain
> error conditions (see Section 8.2) or adjustments from the
> SCTP user.
>
> When there is outbound data to send and the primary path
> becomes inactive (e.g., due to failures), or where the SCTP
> user explicitly requests to send data to an inactive
> destination transport address, before reporting an error to
> its ULP, the SCTP endpoint should try to send the data to an
> alternate *active* destination transport address if one exists.
>
> When retransmitting data that timed out, if the endpoint is
> multihomed, it should consider each source-destination address
> pair in its retransmission selection policy. When retransmitting
> timed-out data, the endpoint should attempt to pick the most
> divergent source-destination pair from the original
> source-destination pair to which the packet was transmitted.
>
> Note: Rules for picking the most divergent source-destination
> pair are an implementation decision and are not specified
> within this document.
>
> So, we should first reconsider to take the current active
> retransmission transport if we cannot find an alternative
> active one, as otherwise, by sending a user message to an
> inactive destination transport address while excluding an
> active destination transport address, we would not comply
> to RFC4960. If all of that fails, we can still round robin
> through unkown, partial failover, and inactive ones in the
> hope to find something still suitable/useful.
>
> Commit 4141ddc02a92 ("sctp: retran_path update bug fix") broke
> that behaviour by selecting the next non-active transport when
> no other active transport was found besides the current assoc's
> peer.retran_path. Before commit 4141ddc02a92, we would have
> traversed through the list until we reach our peer.retran_path
> again, and in case that is still in state SCTP_ACTIVE, we would
> take it and return. Only if that is not the case either, we
> take the next inactive transport. Besides all that, another
> issue is that transports in state SCTP_UNKNOWN could be preferred
> over transports in state SCTP_ACTIVE in case a SCTP_ACTIVE
> transport appears after SCTP_UNKNOWN in the transport list
> yielding a "weaker" transport state to be used in retransmission.
>
> This patch mostly reverts 4141ddc02a92, but also rewrites
> this function to introduce more clarity and strictness into
> the code. A strict priority of transport states is enforced
> in this patch, hence selection is active > unkown > partial
> failover > inactive.
>
> Fixes: 4141ddc02a92 ("sctp: retran_path update bug fix")
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> Cc: Gui Jianfeng <guijianfeng@cn.fujitsu.com>
> ---
> net/sctp/associola.c | 123 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 73 insertions(+), 50 deletions(-)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index f558433..bac47a4 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -1239,78 +1239,101 @@ void sctp_assoc_update(struct sctp_association *asoc,
> }
>
> /* Update the retran path for sending a retransmitted packet.
> - * Round-robin through the active transports, else round-robin
> - * through the inactive transports as this is the next best thing
> - * we can try.
> + * See also RFC4960, 6.4. Multi-Homed SCTP Endpoints:
> + *
> + * When there is outbound data to send and the primary path
> + * becomes inactive (e.g., due to failures), or where the
> + * SCTP user explicitly requests to send data to an
> + * inactive destination transport address, before reporting
> + * an error to its ULP, the SCTP endpoint should try to send
> + * the data to an alternate active destination transport
> + * address if one exists.
> + *
> + * When retransmitting data that timed out, if the endpoint
> + * is multihomed, it should consider each source-destination
> + * address pair in its retransmission selection policy.
> + * When retransmitting timed-out data, the endpoint should
> + * attempt to pick the most divergent source-destination
> + * pair from the original source-destination pair to which
> + * the packet was transmitted.
> + *
> + * Note: Rules for picking the most divergent source-destination
> + * pair are an implementation decision and are not specified
> + * within this document.
> + *
> + * Our basic strategy is to round-robin transports in priorities
> + * according to sctp_state_prio_map[] e.g., if no such
> + * transport with state SCTP_ACTIVE exists, round-robin through
> + * SCTP_UNKNOWN, etc. You get the picture.
> */
> -void sctp_assoc_update_retran_path(struct sctp_association *asoc)
> +static const u8 sctp_trans_state_to_prio_map[] = {
> + [SCTP_ACTIVE] = 3, /* best case */
> + [SCTP_UNKNOWN] = 2,
> + [SCTP_PF] = 1,
> + [SCTP_INACTIVE] = 0, /* worst case */
> +};
> +
> +static u8 sctp_trans_score(const struct sctp_transport *trans)
> {
> - struct sctp_transport *t, *next;
> - struct list_head *head = &asoc->peer.transport_addr_list;
> - struct list_head *pos;
> + return sctp_trans_state_to_prio_map[trans->state];
> +}
>
> - if (asoc->peer.transport_count == 1)
> - return;
> +static struct sctp_transport *sctp_trans_elect_best(struct sctp_transport *curr,
> + struct sctp_transport *best)
> +{
> + if (best == NULL)
> + return curr;
>
> - /* Find the next transport in a round-robin fashion. */
> - t = asoc->peer.retran_path;
> - pos = &t->transports;
> - next = NULL;
> + return sctp_trans_score(curr) > sctp_trans_score(best) ? curr : best;
> +}
>
> - while (1) {
> - /* Skip the head. */
> - if (pos->next == head)
> - pos = head->next;
> - else
> - pos = pos->next;
> +void sctp_assoc_update_retran_path(struct sctp_association *asoc)
> +{
> + struct sctp_transport *trans = asoc->peer.retran_path;
> + struct sctp_transport *trans_next = NULL;
>
> - t = list_entry(pos, struct sctp_transport, transports);
> + /* We're done as we only have the one and only path. */
> + if (asoc->peer.transport_count == 1)
> + return;
>
> - /* We have exhausted the list, but didn't find any
> - * other active transports. If so, use the next
> - * transport.
> - */
> - if (t == asoc->peer.retran_path) {
> - t = next;
> + /* Iterate from retran_path's successor back to retran_path. */
> + for (trans = list_next_entry(trans, transports); 1;
> + trans = list_next_entry(trans, transports)) {
> + /* Manually skip the head element. */
> + if (&trans->transports == &asoc->peer.transport_addr_list)
> + continue;
> + if (trans->state == SCTP_UNCONFIRMED)
> + continue;
> + trans_next = sctp_trans_elect_best(trans, trans_next);
> + /* Active is good enough for immediate return. */
> + if (trans_next->state == SCTP_ACTIVE)
> break;
> - }
> -
> - /* Try to find an active transport. */
> -
> - if ((t->state == SCTP_ACTIVE) ||
> - (t->state == SCTP_UNKNOWN)) {
> + /* We've reached the end, time to update path. */
> + if (trans == asoc->peer.retran_path)
> break;
> - } else {
> - /* Keep track of the next transport in case
> - * we don't find any active transport.
> - */
> - if (t->state != SCTP_UNCONFIRMED && !next)
> - next = t;
> - }
> }
>
> - if (t)
> - asoc->peer.retran_path = t;
> - else
> - t = asoc->peer.retran_path;
> + if (trans_next != NULL)
> + asoc->peer.retran_path = trans_next;
>
> - pr_debug("%s: association:%p addr:%pISpc\n", __func__, asoc,
> - &t->ipaddr.sa);
> + pr_debug("%s: association:%p updated new path to addr:%pISpc\n",
> + __func__, asoc, &asoc->peer.retran_path->ipaddr.sa);
> }
>
> -/* Choose the transport for sending retransmit packet. */
> -struct sctp_transport *sctp_assoc_choose_alter_transport(
> - struct sctp_association *asoc, struct sctp_transport *last_sent_to)
> +struct sctp_transport *
> +sctp_assoc_choose_alter_transport(struct sctp_association *asoc,
> + struct sctp_transport *last_sent_to)
> {
> /* If this is the first time packet is sent, use the active path,
> * else use the retran path. If the last packet was sent over the
> * retran path, update the retran path and use it.
> */
> - if (!last_sent_to)
> + if (last_sent_to == NULL) {
> return asoc->peer.active_path;
> - else {
> + } else {
> if (last_sent_to == asoc->peer.retran_path)
> sctp_assoc_update_retran_path(asoc);
> +
> return asoc->peer.retran_path;
> }
> }
> --
> 1.8.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* RE: [PATCH net] net: sctp: fix multihoming retransmission path selection to rfc4960
From: David Laight @ 2014-02-20 12:25 UTC (permalink / raw)
To: 'Daniel Borkmann', davem@davemloft.net
Cc: netdev@vger.kernel.org, linux-sctp@vger.kernel.org, Gui Jianfeng
In-Reply-To: <1392897186-26841-1-git-send-email-dborkman@redhat.com>
From: Daniel Borkmann
>
> Problem statement: 1) both paths (primary path1 and alternate
> path2) are up after the association has been established i.e.,
> HB packets are normally exchanged, 2) path2 gets inactive after
> path_max_retrans * max_rto timed out (i.e. path2 is down completely),
> 3) now, if a transmission times out on the only surviving/active
> path1 (any ~1sec network service impact could cause this like
> a channel bonding failover), then the retransmitted packets are
> sent over the inactive path2; this happens with partial failover
> and without it.
>
> Besides not being optimal in the above scenario, a small failure
> or timeout in the only existing path has the potential to cause
> long delays in the retransmission (depending on RTO_MAX) until
> the still active path is reselected.
The current behaviour doesn't seem very good - real networks tend
to have non-zero packet loss these days (for all sorts of reasons).
I guess that under moderate traffic flow retransmit requests from
the remote system recover the data before a timeout actually occurs.
That probably means that a path with a high error rate will continue
to be used when an alternate path would be much better.
I was wondering whether it is valid (or even reasonable) to send
the retransmit down multiple paths? Particularly if they are
not known to be working.
Or maybe resend heartbeats in a desperate attempt to find a working
path?
Do you guys know which kernel version(s) have that patch?
We have a few customers using sctp (for m3ua) and I really ought
to keep track of the 'good' and 'bad' kernel versions.
David
^ permalink raw reply
* Re: [PATCH net] net: sctp: fix multihoming retransmission path selection to rfc4960
From: Neil Horman @ 2014-02-20 12:40 UTC (permalink / raw)
To: David Laight
Cc: 'Daniel Borkmann', davem@davemloft.net,
netdev@vger.kernel.org, linux-sctp@vger.kernel.org, Gui Jianfeng
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6C762B@AcuExch.aculab.com>
On Thu, Feb 20, 2014 at 12:25:21PM +0000, David Laight wrote:
> From: Daniel Borkmann
> >
> > Problem statement: 1) both paths (primary path1 and alternate
> > path2) are up after the association has been established i.e.,
> > HB packets are normally exchanged, 2) path2 gets inactive after
> > path_max_retrans * max_rto timed out (i.e. path2 is down completely),
> > 3) now, if a transmission times out on the only surviving/active
> > path1 (any ~1sec network service impact could cause this like
> > a channel bonding failover), then the retransmitted packets are
> > sent over the inactive path2; this happens with partial failover
> > and without it.
> >
> > Besides not being optimal in the above scenario, a small failure
> > or timeout in the only existing path has the potential to cause
> > long delays in the retransmission (depending on RTO_MAX) until
> > the still active path is reselected.
>
> The current behaviour doesn't seem very good - real networks tend
> to have non-zero packet loss these days (for all sorts of reasons).
>
> I guess that under moderate traffic flow retransmit requests from
> the remote system recover the data before a timeout actually occurs.
>
> That probably means that a path with a high error rate will continue
> to be used when an alternate path would be much better.
>
Not really sure what you mean here. Why would we use a path with a high error
rate when another one would be much better. If we get to many retransmits on
the current active path, we select a different one, attempting to use collected
metrics to determine which path would be the most prefereable.
> I was wondering whether it is valid (or even reasonable) to send
> the retransmit down multiple paths? Particularly if they are
> not known to be working.
Yes, quick failover defines that behavior:
http://tools.ietf.org/html/draft-nishida-tsvwg-sctp-failover-05
And if its not appropriate for your network, you can disable it via sysctl.
> Or maybe resend heartbeats in a desperate attempt to find a working
> path?
>
> Do you guys know which kernel version(s) have that patch?
Which patch, what daniel describes above has been the behavior for some time
IIRC.
> We have a few customers using sctp (for m3ua) and I really ought
> to keep track of the 'good' and 'bad' kernel versions.
>
> David
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" 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] net: sctp: fix multihoming retransmission path selection to rfc4960
From: Daniel Borkmann @ 2014-02-20 12:48 UTC (permalink / raw)
To: David Laight
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-sctp@vger.kernel.org, Gui Jianfeng
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6C762B@AcuExch.aculab.com>
On 02/20/2014 01:25 PM, David Laight wrote:
> From: Daniel Borkmann
>>
>> Problem statement: 1) both paths (primary path1 and alternate
>> path2) are up after the association has been established i.e.,
>> HB packets are normally exchanged, 2) path2 gets inactive after
>> path_max_retrans * max_rto timed out (i.e. path2 is down completely),
>> 3) now, if a transmission times out on the only surviving/active
>> path1 (any ~1sec network service impact could cause this like
>> a channel bonding failover), then the retransmitted packets are
>> sent over the inactive path2; this happens with partial failover
>> and without it.
>>
>> Besides not being optimal in the above scenario, a small failure
>> or timeout in the only existing path has the potential to cause
>> long delays in the retransmission (depending on RTO_MAX) until
>> the still active path is reselected.
>
> The current behaviour doesn't seem very good - real networks tend
> to have non-zero packet loss these days (for all sorts of reasons).
>
> I guess that under moderate traffic flow retransmit requests from
> the remote system recover the data before a timeout actually occurs.
>
> That probably means that a path with a high error rate will continue
> to be used when an alternate path would be much better.
>
> I was wondering whether it is valid (or even reasonable) to send
> the retransmit down multiple paths? Particularly if they are
> not known to be working.
As far as I can see, the RFC says that we should pick one, and
not broadcast through all paths, besides HB should monitor these
anyway.
Future work, however, could select a retransmission path "more
intelligent" based on further transport path properties, but
that is certainly not net material, plus it seems we would need
additional state logic indicating that a path has been used before
to not exclude other less optimal transports on successive
retransmits.
> Or maybe resend heartbeats in a desperate attempt to find a working
> path?
Yes, that is done through HBs, see 1.5.7 of RFC4960.
> Do you guys know which kernel version(s) have that patch?
git describe 4141ddc02a92
v2.6.26-rc4-210-g4141ddc
> We have a few customers using sctp (for m3ua) and I really ought
> to keep track of the 'good' and 'bad' kernel versions.
>
> David
^ permalink raw reply
* [PATCH 0/4] net: rfkill: gpio: clean up and a few new acpi ids
From: Heikki Krogerus @ 2014-02-20 12:51 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: Chen-Yu Tsai, Rhyland Klein, linux-wireless, netdev, linux-kernel
Hi,
I was waiting for the DT support from Chen-Yu before sending these,
but decided it makes no difference when I send them. I'm dropping the
con ID in the second patch because Dan noticed the warning, but of
course it will mean the "gpios" property can be used with DT.
The two last patches just add ACPI IDs for some new Baytrail based
boards.
Heikki Krogerus (4):
net: rfkill: gpio: remove unused and obsolete platform parameters
net: rfkill: gpio: remove gpio names
net: rfkill: gpio: add ACPI ID for GPS module on Lenove Miix2
net: rfkill: gpio: add ACPI IDs for a Broadcom bluetooth chip
include/linux/rfkill-gpio.h | 10 ----------
net/rfkill/rfkill-gpio.c | 38 +++++++-------------------------------
2 files changed, 7 insertions(+), 41 deletions(-)
--
1.9.0.rc3
^ permalink raw reply
* [PATCH 1/4] net: rfkill: gpio: remove unused and obsolete platform parameters
From: Heikki Krogerus @ 2014-02-20 12:51 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: Chen-Yu Tsai, Rhyland Klein, linux-wireless, netdev, linux-kernel
In-Reply-To: <1392900697-27577-1-git-send-email-heikki.krogerus@linux.intel.com>
After upgrading to descriptor based gpios, the gpio numbers
are not used anymore. The power_clk_name and the platform
specific setup and close hooks are not used by anybody, and
we should not encourage use of such things, so removing them.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
include/linux/rfkill-gpio.h | 10 ----------
net/rfkill/rfkill-gpio.c | 15 +--------------
2 files changed, 1 insertion(+), 24 deletions(-)
diff --git a/include/linux/rfkill-gpio.h b/include/linux/rfkill-gpio.h
index 4d09f6e..20bcb55 100644
--- a/include/linux/rfkill-gpio.h
+++ b/include/linux/rfkill-gpio.h
@@ -27,21 +27,11 @@
* struct rfkill_gpio_platform_data - platform data for rfkill gpio device.
* for unused gpio's, the expected value is -1.
* @name: name for the gpio rf kill instance
- * @reset_gpio: GPIO which is used for reseting rfkill switch
- * @shutdown_gpio: GPIO which is used for shutdown of rfkill switch
- * @power_clk_name: [optional] name of clk to turn off while blocked
- * @gpio_runtime_close: clean up platform specific gpio configuration
- * @gpio_runtime_setup: set up platform specific gpio configuration
*/
struct rfkill_gpio_platform_data {
char *name;
- int reset_gpio;
- int shutdown_gpio;
- const char *power_clk_name;
enum rfkill_type type;
- void (*gpio_runtime_close)(struct platform_device *);
- int (*gpio_runtime_setup)(struct platform_device *);
};
#endif /* __RFKILL_GPIO_H */
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index bd2a5b9..0adda44 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -87,7 +87,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
{
struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
struct rfkill_gpio_data *rfkill;
- const char *clk_name = NULL;
struct gpio_desc *gpio;
int ret;
int len;
@@ -101,7 +100,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
if (ret)
return ret;
} else if (pdata) {
- clk_name = pdata->power_clk_name;
rfkill->name = pdata->name;
rfkill->type = pdata->type;
} else {
@@ -120,7 +118,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
snprintf(rfkill->reset_name, len + 6 , "%s_reset", rfkill->name);
snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", rfkill->name);
- rfkill->clk = devm_clk_get(&pdev->dev, clk_name);
+ rfkill->clk = devm_clk_get(&pdev->dev, NULL);
gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
if (!IS_ERR(gpio)) {
@@ -146,14 +144,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
return -EINVAL;
}
- if (pdata && pdata->gpio_runtime_setup) {
- ret = pdata->gpio_runtime_setup(pdev);
- if (ret) {
- dev_err(&pdev->dev, "can't set up gpio\n");
- return ret;
- }
- }
-
rfkill->rfkill_dev = rfkill_alloc(rfkill->name, &pdev->dev,
rfkill->type, &rfkill_gpio_ops,
rfkill);
@@ -174,10 +164,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
static int rfkill_gpio_remove(struct platform_device *pdev)
{
struct rfkill_gpio_data *rfkill = platform_get_drvdata(pdev);
- struct rfkill_gpio_platform_data *pdata = pdev->dev.platform_data;
- if (pdata && pdata->gpio_runtime_close)
- pdata->gpio_runtime_close(pdev);
rfkill_unregister(rfkill->rfkill_dev);
rfkill_destroy(rfkill->rfkill_dev);
--
1.9.0.rc3
^ permalink raw reply related
* [PATCH 2/4] net: rfkill: gpio: remove gpio names
From: Heikki Krogerus @ 2014-02-20 12:51 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: Chen-Yu Tsai, Rhyland Klein, linux-wireless, netdev, linux-kernel,
Arnd Bergmann, Linus Walleij, Alexandre Courbot
In-Reply-To: <1392900697-27577-1-git-send-email-heikki.krogerus@linux.intel.com>
There is no use for them in this driver. This will fix a
static checker warning..
net/rfkill/rfkill-gpio.c:144 rfkill_gpio_probe()
warn: variable dereferenced before check 'rfkill->name'
This will also make sure that when DT support is added,
"gpios" property can be used as no con_id labels are
provided.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
net/rfkill/rfkill-gpio.c | 19 ++-----------------
1 file changed, 2 insertions(+), 17 deletions(-)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index 0adda44..ad5e354 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -36,8 +36,6 @@ struct rfkill_gpio_data {
struct gpio_desc *shutdown_gpio;
struct rfkill *rfkill_dev;
- char *reset_name;
- char *shutdown_name;
struct clk *clk;
bool clk_enabled;
@@ -89,7 +87,6 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
struct rfkill_gpio_data *rfkill;
struct gpio_desc *gpio;
int ret;
- int len;
rfkill = devm_kzalloc(&pdev->dev, sizeof(*rfkill), GFP_KERNEL);
if (!rfkill)
@@ -106,21 +103,9 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
return -ENODEV;
}
- len = strlen(rfkill->name);
- rfkill->reset_name = devm_kzalloc(&pdev->dev, len + 7, GFP_KERNEL);
- if (!rfkill->reset_name)
- return -ENOMEM;
-
- rfkill->shutdown_name = devm_kzalloc(&pdev->dev, len + 10, GFP_KERNEL);
- if (!rfkill->shutdown_name)
- return -ENOMEM;
-
- snprintf(rfkill->reset_name, len + 6 , "%s_reset", rfkill->name);
- snprintf(rfkill->shutdown_name, len + 9, "%s_shutdown", rfkill->name);
-
rfkill->clk = devm_clk_get(&pdev->dev, NULL);
- gpio = devm_gpiod_get_index(&pdev->dev, rfkill->reset_name, 0);
+ gpio = devm_gpiod_get_index(&pdev->dev, NULL, 0);
if (!IS_ERR(gpio)) {
ret = gpiod_direction_output(gpio, 0);
if (ret)
@@ -128,7 +113,7 @@ static int rfkill_gpio_probe(struct platform_device *pdev)
rfkill->reset_gpio = gpio;
}
- gpio = devm_gpiod_get_index(&pdev->dev, rfkill->shutdown_name, 1);
+ gpio = devm_gpiod_get_index(&pdev->dev, NULL, 1);
if (!IS_ERR(gpio)) {
ret = gpiod_direction_output(gpio, 0);
if (ret)
--
1.9.0.rc3
^ permalink raw reply related
* [PATCH 3/4] net: rfkill: gpio: add ACPI ID for GPS module on Lenove Miix2
From: Heikki Krogerus @ 2014-02-20 12:51 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: Chen-Yu Tsai, Rhyland Klein, linux-wireless, netdev, linux-kernel
In-Reply-To: <1392900697-27577-1-git-send-email-heikki.krogerus@linux.intel.com>
On Lenovo Miix 2 8", BCM4752 is renamed LNV4752.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
net/rfkill/rfkill-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index ad5e354..ec38884 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -158,6 +158,7 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM4752", RFKILL_TYPE_GPS },
+ { "LNV4752", RFKILL_TYPE_GPS },
{ },
};
--
1.9.0.rc3
^ permalink raw reply related
* [PATCH 4/4] net: rfkill: gpio: add ACPI IDs for a Broadcom bluetooth chip
From: Heikki Krogerus @ 2014-02-20 12:51 UTC (permalink / raw)
To: Johannes Berg, David S. Miller
Cc: Chen-Yu Tsai, Rhyland Klein, linux-wireless, netdev, linux-kernel
In-Reply-To: <1392900697-27577-1-git-send-email-heikki.krogerus@linux.intel.com>
This adds ACPI IDs for Broadcom bluetooth chip BCM43241 used
on various Baytrail based boards such as Lenovo Miix 2 and
Asus Transformer Book T100TA.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
net/rfkill/rfkill-gpio.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index ec38884..3a38861 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -157,6 +157,9 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
}
static const struct acpi_device_id rfkill_acpi_match[] = {
+ { "BCM2E1A", RFKILL_TYPE_BLUETOOTH },
+ { "BCM2E39", RFKILL_TYPE_BLUETOOTH },
+ { "BCM2E3D", RFKILL_TYPE_BLUETOOTH },
{ "BCM4752", RFKILL_TYPE_GPS },
{ "LNV4752", RFKILL_TYPE_GPS },
{ },
--
1.9.0.rc3
^ permalink raw reply related
* [PATCH] net: sctp: Potentially-Failed state should not be reached from unconfirmed state
From: Matija Glavinic Pecotic @ 2014-02-20 13:13 UTC (permalink / raw)
To: linux-sctp@vger.kernel.org; +Cc: netdev@vger.kernel.org
In current implementation it is possible to reach PF state from unconfirmed.
We can interpret sctp-failover-02 in a way that PF state is meant to be reached
only from active state, in the end, this is when entering PF state makes sense.
Here are few quotes from sctp-failover-02, but regardless of these, same
understanding can be reached from whole section 5:
Section 5.1, quickfailover guide:
"The PF state is an intermediate state between Active and Failed states."
"Each time the T3-rtx timer expires on an active or idle
destination, the error counter of that destination address will
be incremented. When the value in the error counter exceeds
PFMR, the endpoint should mark the destination transport address as PF."
There are several concrete reasons for such interpretation. For start, rfc4960
does not take into concern quickfailover algorithm. Therefore, quickfailover
must comply to 4960. Point where this compliance can be argued is following
behavior:
When PF is entered, association overall error counter is incremented for each
missed HB. This is contradictory to rfc4960, as address, while in unconfirmed
state, is subjected to probing, and while it is probed, it should not increment
association overall error counter. This has as a consequence that we might end
up in situation in which we drop association due path failure on unconfirmed
address, in case we have wrong configuration in a way:
Association.Max.Retrans == Path.Max.Retrans.
Another reason is that entering PF from unconfirmed will cause a loss of address
confirmed event when address is once (if) confirmed. This is fine from failover
guide point of view, but it is not consistent with behavior preceding failover
implementation and recommendation from 4960:
5.4. Path Verification
Whenever a path is confirmed, an indication MAY be given to the upper
layer.
Signed-off-by: Matija Glavinic Pecotic <matija.glavinic-pecotic.ext@nsn.com>
--- net-next.orig/net/sctp/sm_sideeffect.c
+++ net-next/net/sctp/sm_sideeffect.c
@@ -495,11 +495,12 @@ static void sctp_do_8_2_transport_strike
}
/* If the transport error count is greater than the pf_retrans
- * threshold, and less than pathmaxrtx, then mark this transport
- * as Partially Failed, ee SCTP Quick Failover Draft, secon 5.1,
- * point 1
+ * threshold, and less than pathmaxrtx, and if the current state
+ * is not SCTP_UNCONFIRMED, then mark this transport as Partially
+ * Failed, see SCTP Quick Failover Draft, section 5.1
*/
if ((transport->state != SCTP_PF) &&
+ (transport->state != SCTP_UNCONFIRMED) &&
(asoc->pf_retrans < transport->pathmaxrxt) &&
(transport->error_count > asoc->pf_retrans)) {
^ permalink raw reply
* Re: [Xen-devel] [RFC v2 1/4] bridge: enable interfaces to opt out from becoming the root bridge
From: Zoltan Kiss @ 2014-02-20 13:19 UTC (permalink / raw)
To: Luis R. Rodriguez, Stephen Hemminger
Cc: Ian Campbell, kvm, netdev@vger.kernel.org, bridge,
linux-kernel@vger.kernel.org, xen-devel
In-Reply-To: <CAB=NE6X6Vuo3iib0W-c5cxv0QBpnZtCC0sFyuULugQoEZAbRtg@mail.gmail.com>
On 19/02/14 17:02, Luis R. Rodriguez wrote:
> On Wed, Feb 19, 2014 at 6:35 AM, Zoltan Kiss <zoltan.kiss@citrix.com> wrote:
>> On 19/02/14 09:52, Ian Campbell wrote:
>>> Can't we arrange things in the Xen hotplug scripts such that if the
>>> root_block stuff isn't available/doesn't work we fallback to the
>>> existing fe:ff:ff:ff:ff usage?
>>>
>>> That would avoid concerns about forward/backwards compat I think. It
>>> wouldn't solve the issue you are targeting on old systems, but it also
>>> doesn't regress them any further.
>>
>> I agree, I think this problem could be better handled from userspace: if it
>> can set root_block then change the default MAC to a random one, if it can't,
>> then stay with the default one. Or if someone doesn't care about STP but DAD
>> is still important, userspace can have a force_random_mac option somewhere
>> to change to a random MAC regardless of root_block presence.
>
> Folks, what if I repurpose my patch to use the IFF_BRIDGE_NON_ROOT (or
> relabel to IFF_ROOT_BLOCK_DEF) flag for a default driver preference
> upon initialization so that root block will be used once the device
> gets added to a bridge. The purpose would be to avoid drivers from
> using the high MAC address hack, streamline to use a random MAC
> address thereby avoiding the possible duplicate address situation for
> IPv6. In the STP use case for these interfaces we'd just require
> userspace to unset the root block. I'd consider the STP use case the
> most odd of all. The caveat to this approach is 3.8 would be needed
> (or its the root block patches cherry picked) for base kernels older
> than 3.8.
How about this: netback sets the root_block flag and a random MAC by
default. So the default behaviour won't change, DAD will be happy, and
userspace don't have to do anything unless it's using netback for STP
root bridge (I don't think there are too many toolstacks doing that), in
which case it has to remove the root_block flag instead of setting a
random MAC.
Zoli
^ permalink raw reply
* RE: [PATCH net] net: sctp: fix multihoming retransmission path selection to rfc4960
From: David Laight @ 2014-02-20 13:25 UTC (permalink / raw)
To: 'Daniel Borkmann'
Cc: davem@davemloft.net, netdev@vger.kernel.org,
linux-sctp@vger.kernel.org, Gui Jianfeng
In-Reply-To: <5305F993.4060603@redhat.com>
From: Daniel Borkmann
> On 02/20/2014 01:25 PM, David Laight wrote:
> > I was wondering whether it is valid (or even reasonable) to send
> > the retransmit down multiple paths? Particularly if they are
> > not known to be working.
>
> As far as I can see, the RFC says that we should pick one, and
> not broadcast through all paths, besides HB should monitor these
> anyway.
>
> Future work, however, could select a retransmission path "more
> intelligent" based on further transport path properties, but
> that is certainly not net material, plus it seems we would need
> additional state logic indicating that a path has been used before
> to not exclude other less optimal transports on successive
> retransmits.
Yes, anything like that probably requires a few steps (and testing)
before being finally implemented.
I always like to have a list of 'future work' (mostly in my head).
Sometimes you suddenly think of a cheap way of doing something,
and you also don't want to move in the wrong direction.
> > Or maybe resend heartbeats in a desperate attempt to find a working
> > path?
>
> Yes, that is done through HBs, see 1.5.7 of RFC4960.
I've managed so far without having to read the SCTP RFCs :-)
But I've implemented enough comms protocols over the years to
know most of the pitfalls.
> > Do you guys know which kernel version(s) have that patch?
>
> git describe 4141ddc02a92
> v2.6.26-rc4-210-g4141ddc
Thanks.
David
^ permalink raw reply
* [PATCH ipsec-next] pfkey: fix SADB_X_EXT_FILTER length check
From: Nicolas Dichtel @ 2014-02-20 13:52 UTC (permalink / raw)
To: steffen.klassert, herbert, davem
Cc: netdev, dan.carpenter, kbuild-all, Nicolas Dichtel
In-Reply-To: <20140220100735.GA25752@elgon.mountain>
This patch fixes commit d3623099d350 ("ipsec: add support of limited SA dump").
sadb_ext_min_len array should be updated with the new type (SADB_X_EXT_FILTER).
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
---
net/key/af_key.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index f0879c19f452..a50d979b5926 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -365,6 +365,7 @@ static const u8 sadb_ext_min_len[] = {
[SADB_X_EXT_NAT_T_OA] = (u8) sizeof(struct sadb_address),
[SADB_X_EXT_SEC_CTX] = (u8) sizeof(struct sadb_x_sec_ctx),
[SADB_X_EXT_KMADDRESS] = (u8) sizeof(struct sadb_x_kmaddress),
+ [SADB_X_EXT_FILTER] = (u8) sizeof(struct sadb_x_filter),
};
/* Verify sadb_address_{len,prefixlen} against sa_family. */
--
1.8.5.4
^ permalink raw reply related
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