* [PATCH net-next 2/3] r8152: fix the warnings and a error from checkpatch.pl
From: Hayes Wang @ 2014-01-09 6:50 UTC (permalink / raw)
To: oliver, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1389250232-8663-1-git-send-email-hayeswang@realtek.com>
Fix the following warnings and error:
- WARNING: usb_free_urb(NULL) is safe this check is probably not required
- WARNING: kfree(NULL) is safe this check is probably not required
- ERROR: do not use C99 // comments
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/r8152.c | 42 +++++++++++++++---------------------------
1 file changed, 15 insertions(+), 27 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 38f3c14..eab078b 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1100,40 +1100,28 @@ static void free_all_mem(struct r8152 *tp)
int i;
for (i = 0; i < RTL8152_MAX_RX; i++) {
- if (tp->rx_info[i].urb) {
- usb_free_urb(tp->rx_info[i].urb);
- tp->rx_info[i].urb = NULL;
- }
+ usb_free_urb(tp->rx_info[i].urb);
+ tp->rx_info[i].urb = NULL;
- if (tp->rx_info[i].buffer) {
- kfree(tp->rx_info[i].buffer);
- tp->rx_info[i].buffer = NULL;
- tp->rx_info[i].head = NULL;
- }
+ kfree(tp->rx_info[i].buffer);
+ tp->rx_info[i].buffer = NULL;
+ tp->rx_info[i].head = NULL;
}
for (i = 0; i < RTL8152_MAX_TX; i++) {
- if (tp->tx_info[i].urb) {
- usb_free_urb(tp->tx_info[i].urb);
- tp->tx_info[i].urb = NULL;
- }
+ usb_free_urb(tp->tx_info[i].urb);
+ tp->tx_info[i].urb = NULL;
- if (tp->tx_info[i].buffer) {
- kfree(tp->tx_info[i].buffer);
- tp->tx_info[i].buffer = NULL;
- tp->tx_info[i].head = NULL;
- }
+ kfree(tp->tx_info[i].buffer);
+ tp->tx_info[i].buffer = NULL;
+ tp->tx_info[i].head = NULL;
}
- if (tp->intr_urb) {
- usb_free_urb(tp->intr_urb);
- tp->intr_urb = NULL;
- }
+ usb_free_urb(tp->intr_urb);
+ tp->intr_urb = NULL;
- if (tp->intr_buff) {
- kfree(tp->intr_buff);
- tp->intr_buff = NULL;
- }
+ kfree(tp->intr_buff);
+ tp->intr_buff = NULL;
}
static int alloc_all_mem(struct r8152 *tp)
@@ -2048,7 +2036,7 @@ static void r8153_first_init(struct r8152 *tp)
/* TX share fifo free credit full threshold */
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_TXFIFO_CTRL, TXFIFO_THR_NORMAL2);
- // rx aggregation
+ /* rx aggregation */
ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
ocp_data &= ~RX_AGG_DISABLE;
ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 1/3] r8152: change the descriptor
From: Hayes Wang @ 2014-01-09 6:50 UTC (permalink / raw)
To: oliver, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
In-Reply-To: <1389250232-8663-1-git-send-email-hayeswang@realtek.com>
The r8152 could support RTL8153. Update the relative descriptor.
Signed-off-by: Hayes Wang <hayeswang@realtek.com>
---
drivers/net/usb/Kconfig | 5 +++--
drivers/net/usb/r8152.c | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index 47b0f73..6b638a0 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -92,11 +92,12 @@ config USB_RTL8150
module will be called rtl8150.
config USB_RTL8152
- tristate "Realtek RTL8152 Based USB 2.0 Ethernet Adapters"
+ tristate "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
select MII
help
This option adds support for Realtek RTL8152 based USB 2.0
- 10/100 Ethernet adapters.
+ 10/100 Ethernet adapters and RTL8153 based USB 3.0 10/100/1000
+ Ethernet adapters.
To compile this driver as a module, choose M here: the
module will be called r8152.
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index bf7d549..38f3c14 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -26,7 +26,7 @@
/* Version Information */
#define DRIVER_VERSION "v1.03.0 (2013/12/26)"
#define DRIVER_AUTHOR "Realtek linux nic maintainers <nic_swsd@realtek.com>"
-#define DRIVER_DESC "Realtek RTL8152 Based USB 2.0 Ethernet Adapters"
+#define DRIVER_DESC "Realtek RTL8152/RTL8153 Based USB Ethernet Adapters"
#define MODULENAME "r8152"
#define R8152_PHY_ID 32
--
1.8.4.2
^ permalink raw reply related
* [PATCH net-next 0/3] r8152: behavior modification
From: Hayes Wang @ 2014-01-09 6:50 UTC (permalink / raw)
To: oliver, netdev; +Cc: nic_swsd, linux-kernel, linux-usb, Hayes Wang
The purpose is to add a choice for determining whether add the
limitation between r8152 and ecm drivers or not.
Hayes Wang (3):
r8152: change the descriptor
r8152: fix the warnings and a error from checkpatch.pl
r8152: add supporting the vendor mode only
drivers/net/usb/Kconfig | 14 ++++++++--
drivers/net/usb/cdc_ether.c | 2 +-
drivers/net/usb/r8152.c | 62 ++++++++++++++++++++++-----------------------
drivers/net/usb/r815x.c | 4 +--
4 files changed, 45 insertions(+), 37 deletions(-)
--
1.8.4.2
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael S. Tsirkin @ 2014-01-09 6:48 UTC (permalink / raw)
To: Michael Dalton; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <CANJ5vPLvDff5PMYNGvtt-dd=md8b46ZXAYLv-KnerNcfzDNHpw@mail.gmail.com>
On Wed, Jan 08, 2014 at 07:41:58PM -0800, Michael Dalton wrote:
> Sorry, forgot to mention - if we want to explore combining the buffer
> address and truesize into a single void *, we could also exploit the
> fact that our size ranges from aligned GOOD_PACKET_LEN to PAGE_SIZE, and
> potentially encode fewer values for truesize (and require a smaller
> alignment than 256). The prior e-mails discussion of 256 byte alignment
> with 256 values is just one potential design point.
>
> Best,
>
> Mike
Good point. I think we should keep the option to
make buffers bigger than 4K, so I think we should start with 256
alignment, then see if there are workloads that are improved by smaller
alignment.
Can you add wrapper inline functions to pack/unpack size and
buffer pointer to/from void *?
This way it will be easy to experiment with different alignments.
^ permalink raw reply
* [PATCH net-next] qlcnic: Convert vmalloc/memset to kcalloc
From: Joe Perches @ 2014-01-09 6:42 UTC (permalink / raw)
To: Jitendra Kalsaria; +Cc: linux-driver, netdev, LKML
vmalloc is a limited resource. Don't use it unnecessarily.
It seems this allocation should work with kcalloc.
Remove unnecessary memset(,0,) of buf as it's completely
overwritten as the previously only unset field in
struct qlcnic_pci_func_cfg is now set to 0.
Use kfree instead of vfree.
Use ETH_ALEN instead of 6.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/net/ethernet/qlogic/qlcnic/qlcnic.h | 2 +-
drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 16 ++++++----------
2 files changed, 7 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
index 35d4876..8d7aa4c 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic.h
@@ -1267,7 +1267,7 @@ struct qlcnic_pci_func_cfg {
u16 port_num;
u8 pci_func;
u8 func_state;
- u8 def_mac_addr[6];
+ u8 def_mac_addr[ETH_ALEN];
};
struct qlcnic_npar_func_cfg {
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index b529667..c9b704d 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -6,7 +6,6 @@
*/
#include <linux/slab.h>
-#include <linux/vmalloc.h>
#include <linux/interrupt.h>
#include "qlcnic.h"
@@ -927,38 +926,35 @@ static ssize_t qlcnic_sysfs_read_pci_config(struct file *file,
u32 pci_func_count = qlcnic_get_pci_func_count(adapter);
struct qlcnic_pci_func_cfg *pci_cfg;
struct qlcnic_pci_info *pci_info;
- size_t pci_info_sz, pci_cfg_sz;
+ size_t pci_cfg_sz;
int i, ret;
pci_cfg_sz = pci_func_count * sizeof(*pci_cfg);
if (size != pci_cfg_sz)
return QL_STATUS_INVALID_PARAM;
- pci_info_sz = pci_func_count * sizeof(*pci_info);
- pci_info = vmalloc(pci_info_sz);
+ pci_info = kcalloc(pci_func_count, sizeof(*pci_info), GFP_KERNEL);
if (!pci_info)
return -ENOMEM;
- memset(pci_info, 0, pci_info_sz);
- memset(buf, 0, pci_cfg_sz);
- pci_cfg = (struct qlcnic_pci_func_cfg *)buf;
-
ret = qlcnic_get_pci_info(adapter, pci_info);
if (ret) {
- vfree(pci_info);
+ kfree(pci_info);
return ret;
}
+ pci_cfg = (struct qlcnic_pci_func_cfg *)buf;
for (i = 0; i < pci_func_count; i++) {
pci_cfg[i].pci_func = pci_info[i].id;
pci_cfg[i].func_type = pci_info[i].type;
+ pci_cfg[i].func_state = 0;
pci_cfg[i].port_num = pci_info[i].default_port;
pci_cfg[i].min_bw = pci_info[i].tx_min_bw;
pci_cfg[i].max_bw = pci_info[i].tx_max_bw;
memcpy(&pci_cfg[i].def_mac_addr, &pci_info[i].mac, ETH_ALEN);
}
- vfree(pci_info);
+ kfree(pci_info);
return size;
}
^ permalink raw reply related
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael S. Tsirkin @ 2014-01-09 6:42 UTC (permalink / raw)
To: Michael Dalton; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <CANJ5vP+_aK0PMc6-hwbu6PduVNDe8jbWR19cN5C+k4mWQzdTGg@mail.gmail.com>
On Wed, Jan 08, 2014 at 07:16:18PM -0800, Michael Dalton wrote:
> Hi Michael,
>
> On Wed, Jan 8, 2014 at 5:42 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > Sorry that I didn't notice early, but there seems to be a bug here.
> > See below.
> Yes, that is definitely a bug. Virtio spec permits OOO completions,
> but current code assumes in-order completion. Thanks for catching this.
>
> > Don't need full int really, it's up to 4K/cache line size,
> > 1 byte would be enough, maximum 2 ...
> > So if all we want is extra 1-2 bytes per buffer, we don't really
> > need this extra level of indirection I think.
> > We can just allocate them before the header together with an skb.
> I'm not sure if I'm parsing the above correctly, but do you mean using a
> few bytes at the beginning of the packet buffer to store truesize? I
> think that will break Jason's virtio-net RX frag coalescing
> code. To coalesce consecutive RX packet buffers, our packet buffers must
> be physically adjacent, and any extra bytes before the start of the
> buffer would break that.
>
> We could allocate an SKB per packet buffer, but if we have multi-buffer
> packets often(e.g., netperf benefiting from GSO/GRO), we would be
> allocating 1 SKB per packet buffer instead of 1 SKB per MAX_SKB_FRAGS
> buffers. How do you feel about any of the below alternatives:
>
> (1) Modify the existing mrg_buf_ctx to chain together free entries
> We can use the 'buf' pointer in mergeable_receive_buf_ctx to chain
> together free entries so that we can support OOO completions. This would
> be similar to how virtio-queue manages free sg entries.
>
> (2) Combine the buffer pointer and truesize into a single void* value
> Your point about there only being a byte needed to encode truesize is
> spot on, and I think we could leverage this to eliminate the out-of-band
> metadata ring entirely. If we were willing to change the packet buffer
> alignment from L1_CACHE_BYTES to 256 (or min (256, L1_CACHE_SIZE)),
I think you mean max here.
> we
> could encode the truesize in the least significant 8 bits of the buffer
> address (encoded as truesize >> 8 as we know all sizes are a multiple
> of 256). This would allow packet buffers up to 64KB in length.
>
> Is there another approach you would prefer to any of these? If the
> cleanliness issues and larger alignment aren't too bad, I think (2)
> sounds promising and allow us to eliminate the metadata ring
> entirely while still permitting RX frag coalescing.
>
> Best,
>
> Mike
I agree, this sounds like a better approach. It's certainly no worse than
current net-next code that always allocates about 1.5K,
and struct sk_buff itself is about 256 bytes on 64 bit.
--
MST
^ permalink raw reply
* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Or Gerlitz @ 2014-01-09 6:35 UTC (permalink / raw)
To: Jerry Chu, Eric Dumazet, David Miller
Cc: Eric Dumazet, Herbert Xu, netdev@vger.kernel.org, Yan Burman,
Shlomo Pongratz
In-Reply-To: <CAPshTChSSB8mycXMXSJ656tpBGDqyjUF37+V2S2eAPQs47gJ2A@mail.gmail.com>
On 09/01/2014 05:12, Jerry Chu wrote:
> On Wed, Jan 8, 2014 at 12:02 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
>> On 08/01/2014 00:11, Jerry Chu wrote:
>>> On Tue, Jan 7, 2014 at 12:37 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>>> On Tue, Jan 7, 2014 at 10:32 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>>> wrote:
>>>>> On Tue, 2014-01-07 at 22:19 +0200, Or Gerlitz wrote:
>>>>>> On Tue, Jan 7, 2014 at 6:33 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>>>>> wrote:
>>>>>>> On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>>>>>>>> +
>>>>>>>> +#define MAX_UDP_PORT (1 << 16)
>>>>>>>> +extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
>>>>>>> Thats 512 KB of memory.
>>>>>>> This will greatly impact forwarding performance of UDP packets with
>>>>>>> random ports, and will increase kernel memory size for embedded
>>>>>>> devices.
>>>>>> Re forwarding, are you referring to the case where the forwarded
>>>>>> packets are encapsulated? packets which are not encapusalted will be
>>>>>> flushed in the gro receive handler (this went out by mistake in V2 but
>>>>>> exists in V1) if skb->encapsulation isn't set.
>>>>>>
>>>>> How do you know encapsulation must be tried for a given incoming
>>>>> packet ? NIC do not magically sets skb->encapsulation I think...
>>>> So here's the thing, per my understanding we want to GRO only received
>>>> **encapsulated** packets whose checksum status is != CHECKSUM_NONE
>>> What's wrong with GRO'ing pkts whose csum == CHECKSUM_NONE?
>>
>> I am not sure, intuitively it sounds a bit wrong to me, empirically, it
>> doesn't work for udp encapsulated / vxlan
>> traffic, I got drops from the tcp stack in tcp_rcv_established() -- if
>> GRO-ed packets carry CHECKSUM_NONE
>> we arrive to the csum_error label, which means that
>> tcp_checksum_complete_user() failed for them
> This is odd because if pkts have been aggregated successfully,
> tcp4_gro_receive() should've skb_checksum() and turned CHECKSUM_NONE
> into CHECKSUM_UNNECESSARY. (I think i've already tested this
> case with my GRE-GRO patch on a NIC that sends up pkts w/ CHECKSUM_NONE.
>
> But granted there are a lot of csum related bugs in the current code. I just spent half a day scratching my head on a very low thruput number with my GRE patch over a GRE tunnel w/ csum flag on. I just tracked it down to be buggy TSO/GRE code that will produce bad csum on the tx side.
The "there are a lot of csum related bugs in the current code" comment
sounds really bad, how do we get into a better place? can you point on
the buggy TSO code that produced bad csum on the tx side?
^ permalink raw reply
* Re: [PATCH net-next V3 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-09 6:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: hkchu, edumazet, herbert, netdev, davem, yanb, shlomop
In-Reply-To: <1389219101.31367.21.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/01/2014 00:11, Eric Dumazet wrote:
> On Wed, 2014-01-08 at 22:34 +0200, Or Gerlitz wrote:
>
>> +
>> /* Notify netdevs that UDP port started listening */
>> -static void vxlan_notify_add_rx_port(struct sock *sk)
>> +static void vxlan_notify_add_rx_port(struct vxlan_sock *vs)
>> {
>> struct net_device *dev;
>> + struct sock *sk = vs->sock->sk;
>> struct net *net = sock_net(sk);
>> sa_family_t sa_family = sk->sk_family;
>> __be16 port = inet_sk(sk)->inet_sport;
>> @@ -569,12 +671,16 @@ static void vxlan_notify_add_rx_port(struct sock *sk)
>> port);
>> }
>> rcu_read_unlock();
>> +
>> + if (sa_family == AF_INET)
>> + call_rcu(&vs->rcu, vxlan_add_udp_offload);
> Why waiting RCU grace period here?
Basically the add operation can be done right away, however, since the
delete operation can't be done
instantly when we want it, I wanted to protect against a series of
add/del/add in times T1 < T2 < T3
T1 add(X)
T2 del(X)
T3 add(X)
where the delete is deferred and as a result the 2nd add is done before
the delete and @ the end offload X is not added in the 2nd time.From
your other comment below I conclude that I probably miss something about
the rcu usage here, so will give it further thought.
>
>> }
>>
>> /* Notify netdevs that UDP port is no more listening */
>> -static void vxlan_notify_del_rx_port(struct sock *sk)
>> +static void vxlan_notify_del_rx_port(struct vxlan_sock *vs)
>> {
>> struct net_device *dev;
>> + struct sock *sk = vs->sock->sk;
>> struct net *net = sock_net(sk);
>> sa_family_t sa_family = sk->sk_family;
>> __be16 port = inet_sk(sk)->inet_sport;
>> @@ -586,6 +692,9 @@ static void vxlan_notify_del_rx_port(struct sock *sk)
>> port);
>> }
>> rcu_read_unlock();
>> +
>> + if (sa_family == AF_INET)
>> + call_rcu(&vs->rcu, vxlan_del_udp_offload);
>> }
> This looks buggy.
>
> You need to :
>
> 1) remove the offload structure from list
> 2) Then wait rcu grace period, and finally free the memory
>
>
>
^ permalink raw reply
* RE: [patch] cxgb4: silence shift wrapping static checker warning
From: Dimitrios Michailidis @ 2014-01-09 5:48 UTC (permalink / raw)
To: Dan Carpenter, Kumar A S
Cc: netdev@vger.kernel.org, kernel-janitors@vger.kernel.org
In-Reply-To: <20140109053400.GF1265@elgon.mountain>
> I don't know how large "tp->vlan_shift" is but static checkers worry
> about shift wrapping bugs here.
Indeed.
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Dimitris Michailidis <dm@chelsio.com>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/l2t.c
> b/drivers/net/ethernet/chelsio/cxgb4/l2t.c
> index cb05be905def..81e8402a74b4 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/l2t.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/l2t.c
> @@ -423,7 +423,7 @@ u64 cxgb4_select_ntuple(struct net_device *dev,
> * in the Compressed Filter Tuple.
> */
> if (tp->vlan_shift >= 0 && l2t->vlan != VLAN_NONE)
> - ntuple |= (F_FT_VLAN_VLD | l2t->vlan) << tp->vlan_shift;
> + ntuple |= (u64)(F_FT_VLAN_VLD | l2t->vlan) << tp->vlan_shift;
>
> if (tp->port_shift >= 0)
> ntuple |= (u64)l2t->lport << tp->port_shift;
^ permalink raw reply
* Re: [PATCH net-next V3 3/3] net: Add GRO support for vxlan traffic
From: Or Gerlitz @ 2014-01-09 6:28 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1389218977.31367.18.camel@edumazet-glaptop2.roam.corp.google.com>
On 09/01/2014 00:09, Eric Dumazet wrote:
> On Wed, 2014-01-08 at 22:34 +0200, Or Gerlitz wrote:
>> +
>> +static int vxlan_gro_complete(struct sk_buff *skb, int nhoff)
>> +{
>> + struct ethhdr *eh;
>> + struct packet_offload *ptype;
>> + __be16 type;
>> + /* 22 = 8 bytes for the vlxan header + 14 bytes for the inner eth header */
>> + int vxlan_len = 22;
>
>
> I am pretty sure this can use existing macros or sizeof(...)
sure, will fix
>
^ permalink raw reply
* Re: [PATCH net-next V3 1/3] net: Add GRO support for UDP encapsulating protocols
From: Or Gerlitz @ 2014-01-09 6:25 UTC (permalink / raw)
To: Tom Herbert
Cc: Jerry Chu, Eric Dumazet, Herbert Xu, Linux Netdev List,
David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <CA+mtBx9yKSW5GFz-zrFSDdkshAmGmdVhds5wgwQKJd0SDEdBCw@mail.gmail.com>
On 08/01/2014 23:58, Tom Herbert wrote:
>> +static struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb)
>> >+{
>> >+ struct list_head *ohead = &udp_offload_base;
>> >+ struct udp_offload *poffload;
>> >+ struct sk_buff *p, **pp = NULL;
>> >+ struct udphdr *uh, *uh2;
>> >+ unsigned int hlen, off;
>> >+ int flush = 1;
>> >+
>> >+ if (NAPI_GRO_CB(skb)->udp_mark ||
>> >+ (!skb->encapsulation && skb->ip_summed != CHECKSUM_COMPLETE))
>> >+ goto out;
>> >+
>> >+ /* mark that this skb passed once through the udp gro layer */
>> >+ NAPI_GRO_CB(skb)->udp_mark = 1;
>> >+
>> >+ off = skb_gro_offset(skb);
>> >+ hlen = off + sizeof(*uh);
>> >+ uh = skb_gro_header_fast(skb, off);
>> >+ if (skb_gro_header_hard(skb, hlen)) {
>> >+ uh = skb_gro_header_slow(skb, hlen, off);
>> >+ if (unlikely(!uh))
>> >+ goto out;
>> >+ }
>> >+
>> >+ rcu_read_lock();
>> >+ list_for_each_entry_rcu(poffload, ohead, list) {
>> >+ if (poffload->port != uh->dest || !poffload->callbacks.gro_receive)
> Is gro_receive == NULL ever valid? Maybe we can assert on registration instead of checking on every packet.
I see your point, however, the offload structure contains entries for
both gro and gso, asserting on registration could somehow limit the use
cases, isn't that?
> Maybe make this poffload->port == uh->dest and goto "flush = 0". Check below that list end was reached becomes unnecessary.
Sure, will use goto "flush = 0" and if we didn't go there we'll go to
out_unlock
^ permalink raw reply
* Device tree binding for Managed Switch
From: Tama @ 2014-01-09 6:12 UTC (permalink / raw)
To: netdev@vger.kernel.org
Hi All,
I am new to this mailing this. I am using IMX28 based custom designed board, based on TQ Components TQMA28 module. In our board we are using KSZ8863 and KSZ8895 managed switches instead of standard phy device. How to add support for managed switch in device tree.
I am new to device tree configuration. At present I have got
mac0: ethernet@800f0000 {
compatible = "fsl,imx28-fec";
reg = <0x800f0000 0x4000>;
interrupts = <101>;
status = "disabled";
};
mac1: ethernet@800f4000 {
compatible = "fsl,imx28-fec";
reg = <0x800f4000 0x4000>;
interrupts = <102>;
status = "disabled";
};
In my device tree imx28.dtsi and in imx28-mba28.dts I have got
/* FCC1 management to switch */
eth0: ethernet@800f0000 {
device_type = "network";
compatible = "micrel,ksz8895";
reg = <0x800f0000 0x4000>;
local-mac-address = [ 00 01 02 03 04 07 ];
interrupts = <101>;
fixed-link = <1 1 1000 0 0>;
phy-handle = <&phy0>;
phy0: phy@0 {
fixed-link;
full-duplex;
speed = <1000>;
};
};
/* FCC1 management to switch */
eth1: ethernet@800f4000 {
device_type = "network";
compatible = "micrel, ksz8895";
reg = <0x800f0000 0x4000>;
local-mac-address = [ 00 01 02 03 04 08 ];
interrupts = <102>;
fixed-link = <1 1 1000 0 0>;
phy-handle = <&phy1>;
phy1: phy@1 {
fixed-link;
full-duplex;
speed = <1000>;
};
};
I read threads on this mailing list and modified the device tree for managed switch. I am not 100% sure if my configuration is correct. When the kernel is booting it is scanning for phy devices and as it doesnt get a phy device id it doesnt register any device.
Can some one please share information on how to configure the device tree for device and not a phy.
Thanks
Pritam
^ permalink raw reply
* [patch] cxgb4: silence shift wrapping static checker warning
From: Dan Carpenter @ 2014-01-09 5:34 UTC (permalink / raw)
To: Dimitris Michailidis, Kumar Sanghvi; +Cc: netdev, kernel-janitors
I don't know how large "tp->vlan_shift" is but static checkers worry
about shift wrapping bugs here.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/net/ethernet/chelsio/cxgb4/l2t.c b/drivers/net/ethernet/chelsio/cxgb4/l2t.c
index cb05be905def..81e8402a74b4 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/l2t.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/l2t.c
@@ -423,7 +423,7 @@ u64 cxgb4_select_ntuple(struct net_device *dev,
* in the Compressed Filter Tuple.
*/
if (tp->vlan_shift >= 0 && l2t->vlan != VLAN_NONE)
- ntuple |= (F_FT_VLAN_VLD | l2t->vlan) << tp->vlan_shift;
+ ntuple |= (u64)(F_FT_VLAN_VLD | l2t->vlan) << tp->vlan_shift;
if (tp->port_shift >= 0)
ntuple |= (u64)l2t->lport << tp->port_shift;
^ permalink raw reply related
* Re: [PATCH] netfilter: nf_conntrack: fix RCU race in nf_conntrack_find_get
From: Andrew Vagin @ 2014-01-09 5:24 UTC (permalink / raw)
To: Eric Dumazet, Florian Westphal
Cc: Andrey Vagin, netfilter-devel, netfilter, coreteam, netdev,
linux-kernel, vvs, Pablo Neira Ayuso, Patrick McHardy,
Jozsef Kadlecsik, David S. Miller, Cyrill Gorcunov
In-Reply-To: <1389107305.26646.20.camel@edumazet-glaptop2.roam.corp.google.com>
On Tue, Jan 07, 2014 at 07:08:25AM -0800, Eric Dumazet wrote:
> On Tue, 2014-01-07 at 14:31 +0400, Andrey Vagin wrote:
> > Lets look at destroy_conntrack:
> >
> > hlist_nulls_del_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode);
> > ...
> > nf_conntrack_free(ct)
> > kmem_cache_free(net->ct.nf_conntrack_cachep, ct);
> >
> > net->ct.nf_conntrack_cachep is created with SLAB_DESTROY_BY_RCU.
> >
> > The hash is protected by rcu, so readers look up conntracks without
> > locks.
> > A conntrack is removed from the hash, but in this moment a few readers
> > still can use the conntrack. Then this conntrack is released and another
> > thread creates conntrack with the same address and the equal tuple.
> > After this a reader starts to validate the conntrack:
> > * It's not dying, because a new conntrack was created
> > * nf_ct_tuple_equal() returns true.
> >
> > But this conntrack is not initialized yet, so it can not be used by two
> > threads concurrently. In this case BUG_ON may be triggered from
> > nf_nat_setup_info().
> >
> > Florian Westphal suggested to check the confirm bit too. I think it's
> > right.
> >
> > task 1 task 2 task 3
> > nf_conntrack_find_get
> > ____nf_conntrack_find
> > destroy_conntrack
> > hlist_nulls_del_rcu
> > nf_conntrack_free
> > kmem_cache_free
> > __nf_conntrack_alloc
> > kmem_cache_alloc
> > memset(&ct->tuplehash[IP_CT_DIR_MAX],
> > if (nf_ct_is_dying(ct))
> > if (!nf_ct_tuple_equal()
> >
> > I'm not sure, that I have ever seen this race condition in a real life.
> > Currently we are investigating a bug, which is reproduced on a few node.
> > In our case one conntrack is initialized from a few tasks concurrently,
> > we don't have any other explanation for this.
>
> >
> > Cc: Florian Westphal <fw@strlen.de>
> > Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> > Cc: Patrick McHardy <kaber@trash.net>
> > Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> > Signed-off-by: Andrey Vagin <avagin@openvz.org>
> > ---
> > net/netfilter/nf_conntrack_core.c | 6 +++++-
> > 1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> > index 43549eb..7a34bb2 100644
> > --- a/net/netfilter/nf_conntrack_core.c
> > +++ b/net/netfilter/nf_conntrack_core.c
> > @@ -387,8 +387,12 @@ begin:
> > !atomic_inc_not_zero(&ct->ct_general.use)))
> > h = NULL;
> > else {
> > + /* A conntrack can be recreated with the equal tuple,
> > + * so we need to check that the conntrack is initialized
> > + */
> > if (unlikely(!nf_ct_tuple_equal(tuple, &h->tuple) ||
> > - nf_ct_zone(ct) != zone)) {
> > + nf_ct_zone(ct) != zone) ||
> > + !nf_ct_is_confirmed(ct)) {
> > nf_ct_put(ct);
> > goto begin;
> > }
>
> I do not think this is the right way to fix this problem (if said
> problem is confirmed)
>
> Remember the rule about SLAB_DESTROY_BY_RCU :
>
> When a struct is freed, then reused, its important to set the its refcnt
> (from 0 to 1) only when the structure is fully ready for use.
>
> If a lookup finds a structure which is not yet setup, the
> atomic_inc_not_zero() will fail.
>
> Take a look at sk_clone_lock() and Documentation/RCU/rculist_nulls.txt
>
I have one more question. Looks like I found another problem.
init_conntrack:
hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode,
&net->ct.unconfirmed);
__nf_conntrack_hash_insert:
hlist_nulls_add_head_rcu(&ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode,
&net->ct.hash[hash]);
We use one hlist_nulls_node to add a conntrack into two different lists.
As far as I understand, it's unacceptable in case of
SLAB_DESTROY_BY_RCU.
Lets imagine that we have two threads. The first one enumerates objects
from a first list, the second one recreates an object and insert it in a
second list. If a first thread in this moment stays on the object, it
can read "next", when it's in the second list, so it will continue
to enumerate objects from the second list. It is not what we want, isn't
it?
cpu1 cpu2
hlist_nulls_for_each_entry_rcu(ct)
destroy_conntrack
kmem_cache_free
init_conntrack
hlist_nulls_add_head_rcu
ct = ct->next
^ permalink raw reply
* Re: [PATCH v4 net-next 2/4] sh_eth: Add support for r7s72100
From: Simon Horman @ 2014-01-09 5:03 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: David S. Miller, netdev, linux-sh, linux-arm-kernel, Magnus Damm
In-Reply-To: <52CDBBE1.1010301@cogentembedded.com>
On Wed, Jan 08, 2014 at 11:58:09PM +0300, Sergei Shtylyov wrote:
> On 01/08/2014 11:02 AM, Simon Horman wrote:
>
> >This is a fast ethernet controller.
>
> >Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
>
> [...]
>
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
> >index 4b38533..cc6d4af 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.c
> >+++ b/drivers/net/ethernet/renesas/sh_eth.c
> >@@ -190,6 +190,59 @@ static const u16 sh_eth_offset_fast_rcar[SH_ETH_MAX_REGISTER_OFFSET] = {
> > [TRIMD] = 0x027c,
> > };
> >
> >+static const u16 sh_eth_offset_fast_rz[SH_ETH_MAX_REGISTER_OFFSET] = {
> [...]
> >+ [ECMR] = 0x0500,
> >+ [ECSR] = 0x0510,
> >+ [ECSIPR] = 0x0518,
> >+ [PIR] = 0x0520,
> >+ [APR] = 0x0554,
> >+ [MPR] = 0x0558,
> >+ [PFTCR] = 0x055c,
> >+ [PFRCR] = 0x0560,
> >+ [TPAUSER] = 0x0564,
> >+ [MAHR] = 0x05c0,
> >+ [MALR] = 0x05c8,
> >+ [CEFCR] = 0x0740,
> >+ [FRECR] = 0x0748,
> >+ [TSFRCR] = 0x0750,
> >+ [TLFRCR] = 0x0758,
> >+ [RFCR] = 0x0760,
> >+ [MAFCR] = 0x0778,
>
> You've missed RFLR @ 0x0508. It's a vital register which the
> driver requires to be always mapped.
Thanks, I will fix that.
> >+
> >+ [ARSTR] = 0x0000,
> >+ [TSU_CTRST] = 0x0004,
> >+ [TSU_VTAG0] = 0x0058,
> >+ [TSU_ADSBSY] = 0x0060,
> >+ [TSU_TEN] = 0x0064,
> >+ [TSU_ADRH0] = 0x0100,
> >+ [TSU_ADRL0] = 0x0104,
> >+ [TSU_ADRH31] = 0x01f8,
> >+ [TSU_ADRL31] = 0x01fc,
>
> Looking at the manual, you've missed [TR]X[NA]LCR regs starting
> at offset 0x0080 from TSU block.
Thanks, I will add them.
> I see that both E-MAC and TSU blocks turned out to be different
> from the Gigabit version upon further scrutiny...
>
> >+};
> >+
> > static const u16 sh_eth_offset_fast_sh4[SH_ETH_MAX_REGISTER_OFFSET] = {
> > [ECMR] = 0x0100,
> > [RFLR] = 0x0108,
> [...]
> >@@ -701,6 +762,35 @@ static struct sh_eth_cpu_data r8a7740_data = {
> > .shift_rd0 = 1,
> > };
> >
> >+/* R7S72100 */
> >+static struct sh_eth_cpu_data r7s72100_data = {
> >+ .chip_reset = sh_eth_chip_reset,
> >+ .set_duplex = sh_eth_set_duplex,
> >+
> >+ .register_type = SH_ETH_REG_FAST_RZ,
> >+
> >+ .ecsr_value = ECSR_ICD,
> >+ .ecsipr_value = ECSIPR_ICDIP,
> >+ .eesipr_value = 0xff7f009f,
> >+
> >+ .tx_check = EESR_TC1 | EESR_FTC,
> >+ .eesr_err_check = EESR_TWB1 | EESR_TWB | EESR_TABT | EESR_RABT |
> >+ EESR_RFE | EESR_RDE | EESR_RFRMER | EESR_TFE |
> >+ EESR_TDE | EESR_ECI,
> >+ .fdr_value = 0x0000070f,
> >+ .rmcr_value = RMCR_RNC,
> >+
> >+ .apr = 1,
> >+ .mpr = 1,
> >+ .tpauser = 1,
> >+ .hw_swap = 1,
> >+ .rpadir = 1,
> >+ .rpadir_value = 2 << 16,
> >+ .no_trimd = 1,
> >+ .tsu = 1,
> >+ .shift_rd0 = 1,
>
> Perhaps this field should be renamed to something talking about
> check summing support (since bits 0..15 of RD0 contain a frame check
> sum for those SoCs). Or maybe it should be just merged with the
> 'hw_crc' field...
I have no feelings about that one way or another.
But it seems to be orthogonal to this patch.
> Well, now the comments about your initializer: you've missed to
> set the 'no_psr' field -- this SoC doesn't have PSR (which usually
> holds the LINK signal status). It's not fatal since you're setting
> 'no_ether_link' in the platform data but should be fixed anyway.
> You've also missed to set 'no_ade' field, though 'eesipr_value'
> correctly has EESIPR.ADEIP cleared. And it looks like you've also
> missed to set 'hw_crc' field since this SoC has CSMR...
Thanks, I will add those fields.
> [...]
> >@@ -880,6 +970,8 @@ static unsigned long sh_eth_get_edtrr_trns(struct sh_eth_private *mdp)
> > {
> > if (sh_eth_is_gether(mdp))
> > return EDTRR_TRNS_GETHER;
> >+ else if (sh_eth_is_rz_fast_ether(mdp))
> >+ return EDTRR_TRNS_RZ_ETHER;
>
> I'd just merge this with the GEther case.
Sure, but in that case should we change the name.
As both you and Magnus pointed out to me, the rz is not gigabit.
>
> > else
> > return EDTRR_TRNS_ETHER;
> > }
> [...]
> >@@ -1062,7 +1155,8 @@ static void sh_eth_ring_format(struct net_device *ndev)
> > if (i == 0) {
> > /* Tx descriptor address set */
> > sh_eth_write(ndev, mdp->tx_desc_dma, TDLAR);
> >- if (sh_eth_is_gether(mdp))
> >+ if (sh_eth_is_gether(mdp) ||
> >+ sh_eth_is_rz_fast_ether(mdp))
> > sh_eth_write(ndev, mdp->tx_desc_dma, TDFAR);
>
> Hmm, TDFAR exists also on SH4 Ethers...
Lets fix that separately.
>
> [...]
> >@@ -2564,6 +2666,9 @@ static const u16 *sh_eth_get_register_offset(int register_type)
> > case SH_ETH_REG_FAST_RCAR:
> > reg_offset = sh_eth_offset_fast_rcar;
> > break;
> >+ case SH_ETH_REG_FAST_RZ:
> >+ reg_offset = sh_eth_offset_fast_rz;
> >+ break;
>
> I think it should precede the R-Car case as this chip is newer
> than R-Car and the SoC families appear here in the reverse order.
Sure, will do.
> > case SH_ETH_REG_FAST_SH4:
> > reg_offset = sh_eth_offset_fast_sh4;
> > break;
> [...]
> >diff --git a/drivers/net/ethernet/renesas/sh_eth.h b/drivers/net/ethernet/renesas/sh_eth.h
> >index 0fe35b7..0bcde90 100644
> >--- a/drivers/net/ethernet/renesas/sh_eth.h
> >+++ b/drivers/net/ethernet/renesas/sh_eth.h
> >@@ -156,6 +156,7 @@ enum {
> > enum {
> > SH_ETH_REG_GIGABIT,
> > SH_ETH_REG_FAST_RCAR,
> >+ SH_ETH_REG_FAST_RZ,
>
> I think it should precede the R-Car value.
Sure, will do.
> > SH_ETH_REG_FAST_SH4,
> > SH_ETH_REG_FAST_SH3_SH2
> > };
> >@@ -169,7 +170,7 @@ enum {
> >
> > /* Register's bits
> > */
> >-/* EDSR : sh7734, sh7757, sh7763, and r8a7740 only */
> >+/* EDSR : sh7734, sh7757, sh7763, r8a7740 and r7s72100 only */
>
> Need comma before "and". Sorry for the grammar nitpicking. :-)
Will do.
> > enum EDSR_BIT {
> > EDSR_ENT = 0x01, EDSR_ENR = 0x02,
> > };
> >@@ -191,6 +192,7 @@ enum DMAC_M_BIT {
> > /* EDTRR */
> > enum DMAC_T_BIT {
> > EDTRR_TRNS_GETHER = 0x03,
> >+ EDTRR_TRNS_RZ_ETHER = 0x03,
>
> I doubt we need a special case here. You didn't introduce one for
> the software reset bits.
True, but RZ is not Gigabit. So I think we either need two names
or to choose a more generic name.
>
> > EDTRR_TRNS_ETHER = 0x01,
> > };
>
> WBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 3:41 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <CANJ5vP+_aK0PMc6-hwbu6PduVNDe8jbWR19cN5C+k4mWQzdTGg@mail.gmail.com>
Sorry, forgot to mention - if we want to explore combining the buffer
address and truesize into a single void *, we could also exploit the
fact that our size ranges from aligned GOOD_PACKET_LEN to PAGE_SIZE, and
potentially encode fewer values for truesize (and require a smaller
alignment than 256). The prior e-mails discussion of 256 byte alignment
with 256 values is just one potential design point.
Best,
Mike
^ permalink raw reply
* Re: [PATCH 31/31] staging: r8188eu: Fix smatch error
From: Greg KH @ 2014-01-09 3:18 UTC (permalink / raw)
To: Larry Finger; +Cc: Dan Carpenter, devel, netdev
In-Reply-To: <52C5E5F5.8030200@lwfinger.net>
On Thu, Jan 02, 2014 at 04:19:33PM -0600, Larry Finger wrote:
> On 01/02/2014 02:44 AM, Dan Carpenter wrote:
> >On Sun, Dec 22, 2013 at 05:37:02PM -0600, Larry Finger wrote:
> >>Smatch shows the following:
> >>
> >> CHECK drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> >>drivers/staging/rtl8188eu/core/rtw_mlme_ext.c:1401 OnAssocReq() error: buffer overflow 'pstapriv->sta_aid' 32 <= 32
> >>
> >
> >This is a false positive in Smatch. Don't do work arounds for buggy
> >tools.
> >
> >If you have the cross function database built on the latest version of
> >Smatch then I think it understands the code correctly and doesn't print
> >a warning.
>
> When I analyzed the loop in question, I thought it resulted in a subscript
> of [NUM_STA], but I now see that the largest one is [NUM_STA-1]. I will drop
> this patch in the next round.
>
> Thanks for the info regarding a new version of Smatch. I'll update now.
Ok, I've applied the first 30, but dropped this patch.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael Dalton @ 2014-01-09 3:16 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, lf-virt, Eric Dumazet, David S. Miller
In-Reply-To: <20140109014255.GA19321@redhat.com>
Hi Michael,
On Wed, Jan 8, 2014 at 5:42 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> Sorry that I didn't notice early, but there seems to be a bug here.
> See below.
Yes, that is definitely a bug. Virtio spec permits OOO completions,
but current code assumes in-order completion. Thanks for catching this.
> Don't need full int really, it's up to 4K/cache line size,
> 1 byte would be enough, maximum 2 ...
> So if all we want is extra 1-2 bytes per buffer, we don't really
> need this extra level of indirection I think.
> We can just allocate them before the header together with an skb.
I'm not sure if I'm parsing the above correctly, but do you mean using a
few bytes at the beginning of the packet buffer to store truesize? I
think that will break Jason's virtio-net RX frag coalescing
code. To coalesce consecutive RX packet buffers, our packet buffers must
be physically adjacent, and any extra bytes before the start of the
buffer would break that.
We could allocate an SKB per packet buffer, but if we have multi-buffer
packets often(e.g., netperf benefiting from GSO/GRO), we would be
allocating 1 SKB per packet buffer instead of 1 SKB per MAX_SKB_FRAGS
buffers. How do you feel about any of the below alternatives:
(1) Modify the existing mrg_buf_ctx to chain together free entries
We can use the 'buf' pointer in mergeable_receive_buf_ctx to chain
together free entries so that we can support OOO completions. This would
be similar to how virtio-queue manages free sg entries.
(2) Combine the buffer pointer and truesize into a single void* value
Your point about there only being a byte needed to encode truesize is
spot on, and I think we could leverage this to eliminate the out-of-band
metadata ring entirely. If we were willing to change the packet buffer
alignment from L1_CACHE_BYTES to 256 (or min (256, L1_CACHE_SIZE)), we
could encode the truesize in the least significant 8 bits of the buffer
address (encoded as truesize >> 8 as we know all sizes are a multiple
of 256). This would allow packet buffers up to 64KB in length.
Is there another approach you would prefer to any of these? If the
cleanliness issues and larger alignment aren't too bad, I think (2)
sounds promising and allow us to eliminate the metadata ring
entirely while still permitting RX frag coalescing.
Best,
Mike
^ permalink raw reply
* Re: [PATCH net-next V2 1/3] net: Add GRO support for UDP encapsulating protocols
From: Jerry Chu @ 2014-01-09 3:12 UTC (permalink / raw)
To: Or Gerlitz
Cc: Or Gerlitz, Eric Dumazet, Eric Dumazet, Herbert Xu,
netdev@vger.kernel.org, David Miller, Yan Burman, Shlomo Pongratz
In-Reply-To: <52CD0626.9030001@mellanox.com>
On Wed, Jan 8, 2014 at 12:02 AM, Or Gerlitz <ogerlitz@mellanox.com> wrote:
> On 08/01/2014 00:11, Jerry Chu wrote:
>>
>> On Tue, Jan 7, 2014 at 12:37 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>>
>>> On Tue, Jan 7, 2014 at 10:32 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>> wrote:
>>>>
>>>> On Tue, 2014-01-07 at 22:19 +0200, Or Gerlitz wrote:
>>>>>
>>>>> On Tue, Jan 7, 2014 at 6:33 PM, Eric Dumazet <eric.dumazet@gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Tue, 2014-01-07 at 17:29 +0200, Or Gerlitz wrote:
>>>>>>>
>>>>>>> +
>>>>>>> +#define MAX_UDP_PORT (1 << 16)
>>>>>>> +extern const struct net_offload __rcu *udp_offloads[MAX_UDP_PORT];
>>>>>>
>>>>>> Thats 512 KB of memory.
>>>>>> This will greatly impact forwarding performance of UDP packets with
>>>>>> random ports, and will increase kernel memory size for embedded
>>>>>> devices.
>>>>>
>>>>> Re forwarding, are you referring to the case where the forwarded
>>>>> packets are encapsulated? packets which are not encapusalted will be
>>>>> flushed in the gro receive handler (this went out by mistake in V2 but
>>>>> exists in V1) if skb->encapsulation isn't set.
>>>>>
>>>> How do you know encapsulation must be tried for a given incoming
>>>> packet ? NIC do not magically sets skb->encapsulation I think...
>>>
>>> So here's the thing, per my understanding we want to GRO only received
>>> **encapsulated** packets whose checksum status is != CHECKSUM_NONE
>>
>> What's wrong with GRO'ing pkts whose csum == CHECKSUM_NONE?
>
>
> I am not sure, intuitively it sounds a bit wrong to me, empirically, it
> doesn't work for udp encapsulated / vxlan
> traffic, I got drops from the tcp stack in tcp_rcv_established() -- if
> GRO-ed packets carry CHECKSUM_NONE
> we arrive to the csum_error label, which means that
> tcp_checksum_complete_user() failed for them
This is odd because if pkts have been aggregated successfully,
tcp4_gro_receive() should've skb_checksum() and turned CHECKSUM_NONE
into CHECKSUM_UNNECESSARY. (I think i've already tested this
case with my GRE-GRO patch on a NIC that sends up pkts w/ CHECKSUM_NONE.
But granted there are a lot of csum related bugs in the current code. I just
spent half a day scratching my head on a very low thruput number with
my GRE patch over a GRE tunnel w/ csum flag on. I just tracked it down
to be buggy TSO/GRE code that will produce bad csum on the tx side.
>
>
>
>> Also "udp_offload" is a little misleading - you are not trying to GRO UDP
>> pkts where UDP is the real transport. You are only trying to GRO UDP
>> encapped TCP pkts.
>
>
> Indeed -- however, I just plugged into what was there for GSO, e.g stack
> will not do GSO for plain UDP
> packets, only for those who encapsulate something the code that does this is
> udp_offloads.c -- any suggestion
> how to phrase/frame the change you envision?
There is already udpv4_offload for real udp gso (ufo) offload. How about
"udp_encap_offload" for your stuff?
>
>
>>
>
^ permalink raw reply
* linux-next: manual merge of the ipsec-next tree with the net-next tree
From: Stephen Rothwell @ 2014-01-09 2:41 UTC (permalink / raw)
To: Steffen Klassert, David Miller, netdev
Cc: linux-next, linux-kernel, Daniel Borkmann, Weilong Chen
[-- Attachment #1: Type: text/plain, Size: 605 bytes --]
Hi Steffen,
Today's linux-next merge of the ipsec-next tree got a conflict in
net/xfrm/xfrm_policy.c between commits be7928d20bab ("net: xfrm:
xfrm_policy: fix inline not at beginning of declaration") and
da7c224b1baa ("net: xfrm: xfrm_policy: silence compiler warning") from
the net-next tree and commit 2f3ea9a95c58 ("xfrm: checkpatch erros with
inline keyword position") from the ipsec-next tree.
I fixed it up (I used the version from the net-next tree) and can carry
the fix as necessary (no action is required).
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] virtio-net: auto-tune mergeable rx buffer size for improved performance
From: Michael S. Tsirkin @ 2014-01-09 1:42 UTC (permalink / raw)
To: Michael Dalton; +Cc: netdev, virtualization, Eric Dumazet, David S. Miller
In-Reply-To: <1389072355-20666-3-git-send-email-mwdalton@google.com>
On Mon, Jan 06, 2014 at 09:25:54PM -0800, Michael Dalton wrote:
> Commit 2613af0ed18a ("virtio_net: migrate mergeable rx buffers to page frag
> allocators") changed the mergeable receive buffer size from PAGE_SIZE to
> MTU-size, introducing a single-stream regression for benchmarks with large
> average packet size. There is no single optimal buffer size for all
> workloads. For workloads with packet size <= MTU bytes, MTU + virtio-net
> header-sized buffers are preferred as larger buffers reduce the TCP window
> due to SKB truesize. However, single-stream workloads with large average
> packet sizes have higher throughput if larger (e.g., PAGE_SIZE) buffers
> are used.
>
> This commit auto-tunes the mergeable receiver buffer packet size by
> choosing the packet buffer size based on an EWMA of the recent packet
> sizes for the receive queue. Packet buffer sizes range from MTU_SIZE +
> virtio-net header len to PAGE_SIZE. This improves throughput for
> large packet workloads, as any workload with average packet size >=
> PAGE_SIZE will use PAGE_SIZE buffers.
>
> These optimizations interact positively with recent commit
> ba275241030c ("virtio-net: coalesce rx frags when possible during rx"),
> which coalesces adjacent RX SKB fragments in virtio_net. The coalescing
> optimizations benefit buffers of any size.
>
> Benchmarks taken from an average of 5 netperf 30-second TCP_STREAM runs
> between two QEMU VMs on a single physical machine. Each VM has two VCPUs
> with all offloads & vhost enabled. All VMs and vhost threads run in a
> single 4 CPU cgroup cpuset, using cgroups to ensure that other processes
> in the system will not be scheduled on the benchmark CPUs. Trunk includes
> SKB rx frag coalescing.
>
> net-next w/ virtio_net before 2613af0ed18a (PAGE_SIZE bufs): 14642.85Gb/s
> net-next (MTU-size bufs): 13170.01Gb/s
> net-next + auto-tune: 14555.94Gb/s
>
> Jason Wang also reported a throughput increase on mlx4 from 22Gb/s
> using MTU-sized buffers to about 26Gb/s using auto-tuning.
>
> Signed-off-by: Michael Dalton <mwdalton@google.com>
Sorry that I didn't notice early, but there seems to be a bug here.
See below.
Also, I think we can simplify code, see suggestion below.
> ---
> v2: Add per-receive queue metadata ring to track precise truesize for
> mergeable receive buffers. Remove all truesize approximation. Never
> try to fill a full RX ring (required for metadata ring in v2).
>
> drivers/net/virtio_net.c | 145 ++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 107 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 526dfd8..f6e1ee0 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -26,6 +26,7 @@
> #include <linux/if_vlan.h>
> #include <linux/slab.h>
> #include <linux/cpu.h>
> +#include <linux/average.h>
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -36,11 +37,15 @@ module_param(gso, bool, 0444);
>
> /* FIXME: MTU in config. */
> #define GOOD_PACKET_LEN (ETH_HLEN + VLAN_HLEN + ETH_DATA_LEN)
> -#define MERGE_BUFFER_LEN (ALIGN(GOOD_PACKET_LEN + \
> - sizeof(struct virtio_net_hdr_mrg_rxbuf), \
> - L1_CACHE_BYTES))
> #define GOOD_COPY_LEN 128
>
> +/* Weight used for the RX packet size EWMA. The average packet size is used to
> + * determine the packet buffer size when refilling RX rings. As the entire RX
> + * ring may be refilled at once, the weight is chosen so that the EWMA will be
> + * insensitive to short-term, transient changes in packet size.
> + */
> +#define RECEIVE_AVG_WEIGHT 64
> +
> #define VIRTNET_DRIVER_VERSION "1.0.0"
>
> struct virtnet_stats {
> @@ -65,11 +70,30 @@ struct send_queue {
> char name[40];
> };
>
> +/* Per-packet buffer context for mergeable receive buffers. */
> +struct mergeable_receive_buf_ctx {
> + /* Packet buffer base address. */
> + void *buf;
> +
> + /* Original size of the packet buffer for use in SKB truesize. Does not
> + * include any padding space used to avoid internal fragmentation.
> + */
> + unsigned int truesize;
Don't need full int really, it's up to 4K/cache line size,
1 byte would be enough, maximum 2 ...
So if all we want is extra 1-2 bytes per buffer, we don't really
need this extra level of indirection I think.
We can just allocate them before the header together with an skb.
> +};
> +
> /* Internal representation of a receive virtqueue */
> struct receive_queue {
> /* Virtqueue associated with this receive_queue */
> struct virtqueue *vq;
>
> + /* Circular buffer of mergeable rxbuf contexts. */
> + struct mergeable_receive_buf_ctx *mrg_buf_ctx;
> +
> + /* Number of elements & head index of mrg_buf_ctx. Size must be
> + * equal to the associated virtqueue's vring size.
> + */
> + unsigned int mrg_buf_ctx_size, mrg_buf_ctx_head;
> +
> struct napi_struct napi;
>
> /* Number of input buffers, and max we've ever had. */
> @@ -78,6 +102,9 @@ struct receive_queue {
> /* Chain pages by the private ptr. */
> struct page *pages;
>
> + /* Average packet length for mergeable receive buffers. */
> + struct ewma mrg_avg_pkt_len;
> +
> /* Page frag for packet buffer allocation. */
> struct page_frag alloc_frag;
>
> @@ -327,32 +354,32 @@ err:
>
> static struct sk_buff *receive_mergeable(struct net_device *dev,
> struct receive_queue *rq,
> - void *buf,
> + struct mergeable_receive_buf_ctx *ctx,
> unsigned int len)
> {
> - struct skb_vnet_hdr *hdr = buf;
> + struct skb_vnet_hdr *hdr = ctx->buf;
> int num_buf = hdr->mhdr.num_buffers;
> - struct page *page = virt_to_head_page(buf);
> - int offset = buf - page_address(page);
> - unsigned int truesize = max_t(unsigned int, len, MERGE_BUFFER_LEN);
> + struct page *page = virt_to_head_page(ctx->buf);
> + int offset = ctx->buf - page_address(page);
> + unsigned int truesize = max(len, ctx->truesize);
> +
> struct sk_buff *head_skb = page_to_skb(rq, page, offset, len, truesize);
> struct sk_buff *curr_skb = head_skb;
>
> if (unlikely(!curr_skb))
> goto err_skb;
> -
> while (--num_buf) {
> int num_skb_frags;
>
> - buf = virtqueue_get_buf(rq->vq, &len);
> - if (unlikely(!buf)) {
> + ctx = virtqueue_get_buf(rq->vq, &len);
> + if (unlikely(!ctx)) {
> pr_debug("%s: rx error: %d buffers out of %d missing\n",
> dev->name, num_buf, hdr->mhdr.num_buffers);
> dev->stats.rx_length_errors++;
> goto err_buf;
> }
>
> - page = virt_to_head_page(buf);
> + page = virt_to_head_page(ctx->buf);
> --rq->num;
>
> num_skb_frags = skb_shinfo(curr_skb)->nr_frags;
> @@ -369,13 +396,13 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> head_skb->truesize += nskb->truesize;
> num_skb_frags = 0;
> }
> - truesize = max_t(unsigned int, len, MERGE_BUFFER_LEN);
> + truesize = max(len, ctx->truesize);
> if (curr_skb != head_skb) {
> head_skb->data_len += len;
> head_skb->len += len;
> head_skb->truesize += truesize;
> }
> - offset = buf - page_address(page);
> + offset = ctx->buf - page_address(page);
> if (skb_can_coalesce(curr_skb, num_skb_frags, page, offset)) {
> put_page(page);
> skb_coalesce_rx_frag(curr_skb, num_skb_frags - 1,
> @@ -386,19 +413,20 @@ static struct sk_buff *receive_mergeable(struct net_device *dev,
> }
> }
>
> + ewma_add(&rq->mrg_avg_pkt_len, head_skb->len);
> return head_skb;
>
> err_skb:
> put_page(page);
> while (--num_buf) {
> - buf = virtqueue_get_buf(rq->vq, &len);
> - if (unlikely(!buf)) {
> + ctx = virtqueue_get_buf(rq->vq, &len);
> + if (unlikely(!ctx)) {
> pr_debug("%s: rx error: %d buffers missing\n",
> dev->name, num_buf);
> dev->stats.rx_length_errors++;
> break;
> }
> - page = virt_to_head_page(buf);
> + page = virt_to_head_page(ctx->buf);
> put_page(page);
> --rq->num;
> }
> @@ -419,12 +447,14 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
> pr_debug("%s: short packet %i\n", dev->name, len);
> dev->stats.rx_length_errors++;
> - if (vi->mergeable_rx_bufs)
> - put_page(virt_to_head_page(buf));
> - else if (vi->big_packets)
> + if (vi->mergeable_rx_bufs) {
> + struct mergeable_receive_buf_ctx *ctx = buf;
> + put_page(virt_to_head_page(ctx->buf));
> + } else if (vi->big_packets) {
> give_pages(rq, buf);
> - else
> + } else {
> dev_kfree_skb(buf);
> + }
> return;
> }
>
> @@ -572,29 +602,43 @@ static int add_recvbuf_big(struct receive_queue *rq, gfp_t gfp)
>
> static int add_recvbuf_mergeable(struct receive_queue *rq, gfp_t gfp)
> {
> + const unsigned int ring_size = rq->mrg_buf_ctx_size;
> + const size_t hdr_len = sizeof(struct virtio_net_hdr_mrg_rxbuf);
> struct page_frag *alloc_frag = &rq->alloc_frag;
> - char *buf;
> + struct mergeable_receive_buf_ctx *ctx;
> int err;
> unsigned int len, hole;
>
> - if (unlikely(!skb_page_frag_refill(MERGE_BUFFER_LEN, alloc_frag, gfp)))
> + len = hdr_len + clamp_t(unsigned int, ewma_read(&rq->mrg_avg_pkt_len),
> + GOOD_PACKET_LEN, PAGE_SIZE - hdr_len);
> + len = ALIGN(len, L1_CACHE_BYTES);
> + if (unlikely(!skb_page_frag_refill(len, alloc_frag, gfp)))
> return -ENOMEM;
> - buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> +
> + ctx = &rq->mrg_buf_ctx[rq->mrg_buf_ctx_head];
> + ctx->buf = (char *)page_address(alloc_frag->page) + alloc_frag->offset;
> + ctx->truesize = len;
> get_page(alloc_frag->page);
> - len = MERGE_BUFFER_LEN;
> alloc_frag->offset += len;
> hole = alloc_frag->size - alloc_frag->offset;
> - if (hole < MERGE_BUFFER_LEN) {
> + if (hole < len) {
> + /* To avoid internal fragmentation, if there is very likely not
> + * enough space for another buffer, add the remaining space to
> + * the current buffer. This extra space is not included in
> + * ctx->truesize.
> + */
> len += hole;
> alloc_frag->offset += hole;
> }
>
> - sg_init_one(rq->sg, buf, len);
> - err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, buf, gfp);
> - if (err < 0)
> - put_page(virt_to_head_page(buf));
> -
> - return err;
> + sg_init_one(rq->sg, ctx->buf, len);
> + err = virtqueue_add_inbuf(rq->vq, rq->sg, 1, ctx, gfp);
> + if (err < 0) {
> + put_page(virt_to_head_page(ctx->buf));
> + return err;
> + }
> + rq->mrg_buf_ctx_head = (rq->mrg_buf_ctx_head + 1) & (ring_size - 1);
Wait a second. this assumes that buffers are consumes in order?
This happens to be the case but is not guaranteed by the spec
at all.
> + return 0;
> }
>
> /*
> @@ -610,6 +654,9 @@ static bool try_fill_recv(struct receive_queue *rq, gfp_t gfp)
> int err;
> bool oom;
>
> + /* Do not attempt to add a buffer if the RX ring is full. */
> + if (unlikely(!rq->vq->num_free))
> + return true;
> gfp |= __GFP_COLD;
> do {
> if (vi->mergeable_rx_bufs)
> @@ -1354,8 +1401,10 @@ static void virtnet_free_queues(struct virtnet_info *vi)
> {
> int i;
>
> - for (i = 0; i < vi->max_queue_pairs; i++)
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> netif_napi_del(&vi->rq[i].napi);
> + kfree(vi->rq[i].mrg_buf_ctx);
> + }
>
> kfree(vi->rq);
> kfree(vi->sq);
> @@ -1394,12 +1443,14 @@ static void free_unused_bufs(struct virtnet_info *vi)
> struct virtqueue *vq = vi->rq[i].vq;
>
> while ((buf = virtqueue_detach_unused_buf(vq)) != NULL) {
> - if (vi->mergeable_rx_bufs)
> - put_page(virt_to_head_page(buf));
> - else if (vi->big_packets)
> + if (vi->mergeable_rx_bufs) {
> + struct mergeable_receive_buf_ctx *ctx = buf;
> + put_page(virt_to_head_page(ctx->buf));
> + } else if (vi->big_packets) {
> give_pages(&vi->rq[i], buf);
> - else
> + } else {
> dev_kfree_skb(buf);
> + }
> --vi->rq[i].num;
> }
> BUG_ON(vi->rq[i].num != 0);
> @@ -1509,6 +1560,7 @@ static int virtnet_alloc_queues(struct virtnet_info *vi)
> napi_weight);
>
> sg_init_table(vi->rq[i].sg, ARRAY_SIZE(vi->rq[i].sg));
> + ewma_init(&vi->rq[i].mrg_avg_pkt_len, 1, RECEIVE_AVG_WEIGHT);
> sg_init_table(vi->sq[i].sg, ARRAY_SIZE(vi->sq[i].sg));
> }
>
> @@ -1522,7 +1574,8 @@ err_sq:
>
> static int init_vqs(struct virtnet_info *vi)
> {
> - int ret;
> + struct virtio_device *vdev = vi->vdev;
> + int i, ret;
>
> /* Allocate send & receive queues */
> ret = virtnet_alloc_queues(vi);
> @@ -1533,12 +1586,28 @@ static int init_vqs(struct virtnet_info *vi)
> if (ret)
> goto err_free;
>
> + if (vi->mergeable_rx_bufs) {
> + for (i = 0; i < vi->max_queue_pairs; i++) {
> + struct receive_queue *rq = &vi->rq[i];
> + rq->mrg_buf_ctx_size = virtqueue_get_vring_size(rq->vq);
> + rq->mrg_buf_ctx = kmalloc(sizeof(*rq->mrg_buf_ctx) *
> + rq->mrg_buf_ctx_size,
> + GFP_KERNEL);
> + if (!rq->mrg_buf_ctx) {
> + ret = -ENOMEM;
> + goto err_del_vqs;
> + }
> + }
> + }
> +
> get_online_cpus();
> virtnet_set_affinity(vi);
> put_online_cpus();
>
> return 0;
>
> +err_del_vqs:
> + vdev->config->del_vqs(vdev);
> err_free:
> virtnet_free_queues(vi);
> err:
> --
> 1.8.5.1
^ permalink raw reply
* Re: [PATCH net-next 0/3] bonding: cleanup bond_3ad.c
From: Ding Tianhong @ 2014-01-09 1:36 UTC (permalink / raw)
To: Veaceslav Falico, netdev; +Cc: Jay Vosburgh, Andy Gospodarek
In-Reply-To: <1389196008-28578-1-git-send-email-vfalico@redhat.com>
On 2014/1/8 23:46, Veaceslav Falico wrote:
> It's a huge mess there currently - and, thus, really hard to read and
> debug.
>
> This is the first series, and doesn't change the logic at all, only makes
> it a bit more readable.
>
> 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_3ad.c | 731 +++++++++++++++++++++--------------------
> 1 file changed, 366 insertions(+), 365 deletions(-)
>
Great work!
Tested-by: Ding Tianhong <dingtianhong@huawei.com>
^ permalink raw reply
* [PATCH v4 2/2] ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389227404-12586-1-git-send-email-thaller@redhat.com>
Refactor the deletion/update of prefix routes when removing an
address. Now also consider IFA_F_NOPREFIXROUTE and if there is an address
present with this flag, to not cleanup the route. Instead, assume
that userspace is taking care of this route.
Also perform the same cleanup, when userspace changes an existing address
to add NOPREFIXROUTE (to an address that didn't have this flag). This is
done because when the address was added, a prefix route was created for it.
Since the user now wants to handle this route by himself, we cleanup this
route.
This cleanup of the route is not totally robust. There is no guarantee,
that the route we are about to delete was really the one added by the
kernel. This behavior does not change by the patch, and in practice it
should work just fine.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
net/ipv6/addrconf.c | 184 +++++++++++++++++++++++++++++++---------------------
1 file changed, 109 insertions(+), 75 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7f0f011..f4e2b7a 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -900,15 +900,95 @@ out:
goto out2;
}
+enum cleanup_prefix_rt_t {
+ CLEANUP_PREFIX_RT_NOP, /* no cleanup action for prefix route */
+ CLEANUP_PREFIX_RT_DEL, /* delete the prefix route */
+ CLEANUP_PREFIX_RT_EXPIRE, /* update the lifetime of the prefix route */
+};
+
+/*
+ * Check, whether the prefix for ifp would still need a prefix route
+ * after deleting ifp. The function returns one of the CLEANUP_PREFIX_RT_*
+ * constants.
+ *
+ * 1) we don't purge prefix if address was not permanent.
+ * prefix is managed by its own lifetime.
+ * 2) we also don't purge, if the address was IFA_F_NOPREFIXROUTE.
+ * 3) if there're no addresses, delete prefix.
+ * 4) if there're still other permanent address(es),
+ * corresponding prefix is still permanent.
+ * 5) if there are still other addresses with IFA_F_NOPREFIXROUTE,
+ * don't purge the prefix, assume user space is managing it.
+ * 6) otherwise, update prefix lifetime to the
+ * longest valid lifetime among the corresponding
+ * addresses on the device.
+ * Note: subsequent RA will update lifetime.
+ **/
+static enum cleanup_prefix_rt_t
+check_cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long *expires)
+{
+ struct inet6_ifaddr *ifa;
+ struct inet6_dev *idev = ifp->idev;
+ unsigned long lifetime;
+ enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_DEL;
+
+ *expires = jiffies;
+
+ list_for_each_entry(ifa, &idev->addr_list, if_list) {
+ if (ifa == ifp)
+ continue;
+ if (!ipv6_prefix_equal(&ifa->addr, &ifp->addr,
+ ifp->prefix_len))
+ continue;
+ if (ifa->flags & (IFA_F_PERMANENT | IFA_F_NOPREFIXROUTE))
+ return CLEANUP_PREFIX_RT_NOP;
+
+ action = CLEANUP_PREFIX_RT_EXPIRE;
+
+ spin_lock(&ifa->lock);
+
+ lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
+ /*
+ * Note: Because this address is
+ * not permanent, lifetime <
+ * LONG_MAX / HZ here.
+ */
+ if (time_before(*expires, ifa->tstamp + lifetime * HZ))
+ *expires = ifa->tstamp + lifetime * HZ;
+ spin_unlock(&ifa->lock);
+ }
+
+ return action;
+}
+
+static void
+cleanup_prefix_route(struct inet6_ifaddr *ifp, unsigned long expires, bool del_rt)
+{
+ struct rt6_info *rt;
+
+ rt = addrconf_get_prefix_route(&ifp->addr,
+ ifp->prefix_len,
+ ifp->idev->dev,
+ 0, RTF_GATEWAY | RTF_DEFAULT);
+ if (rt) {
+ if (del_rt)
+ ip6_del_rt(rt);
+ else {
+ if (!(rt->rt6i_flags & RTF_EXPIRES))
+ rt6_set_expires(rt, expires);
+ ip6_rt_put(rt);
+ }
+ }
+}
+
+
/* This function wants to get referenced ifp and releases it before return */
static void ipv6_del_addr(struct inet6_ifaddr *ifp)
{
- struct inet6_ifaddr *ifa, *ifn;
- struct inet6_dev *idev = ifp->idev;
int state;
- int deleted = 0, onlink = 0;
- unsigned long expires = jiffies;
+ enum cleanup_prefix_rt_t action = CLEANUP_PREFIX_RT_NOP;
+ unsigned long expires;
spin_lock_bh(&ifp->state_lock);
state = ifp->state;
@@ -922,7 +1002,7 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
hlist_del_init_rcu(&ifp->addr_lst);
spin_unlock_bh(&addrconf_hash_lock);
- write_lock_bh(&idev->lock);
+ write_lock_bh(&ifp->idev->lock);
if (ifp->flags&IFA_F_TEMPORARY) {
list_del(&ifp->tmp_list);
@@ -933,45 +1013,13 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
__in6_ifa_put(ifp);
}
- list_for_each_entry_safe(ifa, ifn, &idev->addr_list, if_list) {
- if (ifa == ifp) {
- list_del_init(&ifp->if_list);
- __in6_ifa_put(ifp);
+ if (ifp->flags & IFA_F_PERMANENT && !(ifp->flags & IFA_F_NOPREFIXROUTE))
+ action = check_cleanup_prefix_route(ifp, &expires);
- if (!(ifp->flags & IFA_F_PERMANENT) || onlink > 0)
- break;
- deleted = 1;
- continue;
- } else if (ifp->flags & IFA_F_PERMANENT) {
- if (ipv6_prefix_equal(&ifa->addr, &ifp->addr,
- ifp->prefix_len)) {
- if (ifa->flags & IFA_F_PERMANENT) {
- onlink = 1;
- if (deleted)
- break;
- } else {
- unsigned long lifetime;
-
- if (!onlink)
- onlink = -1;
-
- spin_lock(&ifa->lock);
-
- lifetime = addrconf_timeout_fixup(ifa->valid_lft, HZ);
- /*
- * Note: Because this address is
- * not permanent, lifetime <
- * LONG_MAX / HZ here.
- */
- if (time_before(expires,
- ifa->tstamp + lifetime * HZ))
- expires = ifa->tstamp + lifetime * HZ;
- spin_unlock(&ifa->lock);
- }
- }
- }
- }
- write_unlock_bh(&idev->lock);
+ list_del_init(&ifp->if_list);
+ __in6_ifa_put(ifp);
+
+ write_unlock_bh(&ifp->idev->lock);
addrconf_del_dad_timer(ifp);
@@ -979,38 +1027,9 @@ static void ipv6_del_addr(struct inet6_ifaddr *ifp)
inet6addr_notifier_call_chain(NETDEV_DOWN, ifp);
- /*
- * Purge or update corresponding prefix
- *
- * 1) we don't purge prefix here if address was not permanent.
- * prefix is managed by its own lifetime.
- * 2) if there're no addresses, delete prefix.
- * 3) if there're still other permanent address(es),
- * corresponding prefix is still permanent.
- * 4) otherwise, update prefix lifetime to the
- * longest valid lifetime among the corresponding
- * addresses on the device.
- * Note: subsequent RA will update lifetime.
- *
- * --yoshfuji
- */
- if ((ifp->flags & IFA_F_PERMANENT) && onlink < 1) {
- struct rt6_info *rt;
-
- rt = addrconf_get_prefix_route(&ifp->addr,
- ifp->prefix_len,
- ifp->idev->dev,
- 0, RTF_GATEWAY | RTF_DEFAULT);
-
- if (rt) {
- if (onlink == 0) {
- ip6_del_rt(rt);
- rt = NULL;
- } else if (!(rt->rt6i_flags & RTF_EXPIRES)) {
- rt6_set_expires(rt, expires);
- }
- }
- ip6_rt_put(rt);
+ if (action != CLEANUP_PREFIX_RT_NOP) {
+ cleanup_prefix_route(ifp, expires,
+ action == CLEANUP_PREFIX_RT_DEL);
}
/* clean up prefsrc entries */
@@ -3632,6 +3651,7 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
clock_t expires;
unsigned long timeout;
bool was_managetempaddr;
+ bool had_prefixroute;
if (!valid_lft || (prefered_lft > valid_lft))
return -EINVAL;
@@ -3660,6 +3680,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
+ had_prefixroute = ifp->flags & IFA_F_PERMANENT &&
+ !(ifp->flags & IFA_F_NOPREFIXROUTE);
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
IFA_F_NOPREFIXROUTE);
@@ -3675,6 +3697,18 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
expires, flags);
+ } else if (had_prefixroute) {
+ enum cleanup_prefix_rt_t action;
+ unsigned long rt_expires;
+
+ write_lock_bh(&ifp->idev->lock);
+ action = check_cleanup_prefix_route(ifp, &rt_expires);
+ write_unlock_bh(&ifp->idev->lock);
+
+ if (action != CLEANUP_PREFIX_RT_NOP) {
+ cleanup_prefix_route(ifp, rt_expires,
+ action == CLEANUP_PREFIX_RT_DEL);
+ }
}
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v4 1/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <1389227404-12586-1-git-send-email-thaller@redhat.com>
When adding/modifying an IPv6 address, the userspace application needs
a way to suppress adding a prefix route. This is for example relevant
together with IFA_F_MANAGERTEMPADDR, where userspace creates autoconf
generated addresses, but depending on on-link, no route for the
prefix should be added.
Signed-off-by: Thomas Haller <thaller@redhat.com>
---
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 19 +++++++++++++------
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index cfed10b..dea10a8 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -49,6 +49,7 @@ enum {
#define IFA_F_TENTATIVE 0x40
#define IFA_F_PERMANENT 0x80
#define IFA_F_MANAGETEMPADDR 0x100
+#define IFA_F_NOPREFIXROUTE 0x200
struct ifa_cacheinfo {
__u32 ifa_prefered;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31f75ea..7f0f011 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2433,8 +2433,11 @@ static int inet6_addr_add(struct net *net, int ifindex,
valid_lft, prefered_lft);
if (!IS_ERR(ifp)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
+ expires, flags);
+ }
+
/*
* Note that section 3.1 of RFC 4429 indicates
* that the Optimistic flag should not be set for
@@ -3658,7 +3661,8 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
spin_lock_bh(&ifp->lock);
was_managetempaddr = ifp->flags & IFA_F_MANAGETEMPADDR;
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
- IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR);
+ IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+ IFA_F_NOPREFIXROUTE);
ifp->flags |= ifa_flags;
ifp->tstamp = jiffies;
ifp->valid_lft = valid_lft;
@@ -3668,8 +3672,10 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
- expires, flags);
+ if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, ifp->idev->dev,
+ expires, flags);
+ }
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
@@ -3723,7 +3729,8 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
/* We ignore other flags so far. */
- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR;
+ ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
+ IFA_F_NOPREFIXROUTE;
ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
if (ifa == NULL) {
--
1.8.4.2
^ permalink raw reply related
* [PATCH v4 0/2] ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of IP6 routes
From: Thomas Haller @ 2014-01-09 0:30 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Jiri Pirko, netdev, stephen, dcbw, Thomas Haller
In-Reply-To: <20140108200019.GK9007@order.stressinduktion.org>
v1 -> v2: add a second commit, handling NOPREFIXROUTE in ip6_del_addr.
v2 -> v3: reword commit messages, code comments and some refactoring.
v3 -> v4: refactor, rename variables, add enum
Thomas Haller (2):
ipv6 addrconf: add IFA_F_NOPREFIXROUTE flag to suppress creation of
IP6 routes
ipv6 addrconf: don't cleanup prefix route for IFA_F_NOPREFIXROUTE
include/uapi/linux/if_addr.h | 1 +
net/ipv6/addrconf.c | 203 ++++++++++++++++++++++++++-----------------
2 files changed, 123 insertions(+), 81 deletions(-)
--
1.8.4.2
^ 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