* [RFC PATCH v9 00/16] Provide a zero-copy method on KVM virtio-net.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike
We provide an zero-copy method which driver side may get external
buffers to DMA. Here external means driver don't use kernel space
to allocate skb buffers. Currently the external buffer can be from
guest virtio-net driver.
The idea is simple, just to pin the guest VM user space and then
let host NIC driver has the chance to directly DMA to it.
The patches are based on vhost-net backend driver. We add a device
which provides proto_ops as sendmsg/recvmsg to vhost-net to
send/recv directly to/from the NIC driver. KVM guest who use the
vhost-net backend may bind any ethX interface in the host side to
get copyless data transfer thru guest virtio-net frontend.
patch 01-10: net core and kernel changes.
patch 11-13: new device as interface to mantpulate external buffers.
patch 14: for vhost-net.
patch 15: An example on modifying NIC driver to using napi_gro_frags().
patch 16: An example how to get guest buffers based on driver
who using napi_gro_frags().
The guest virtio-net driver submits multiple requests thru vhost-net
backend driver to the kernel. And the requests are queued and then
completed after corresponding actions in h/w are done.
For read, user space buffers are dispensed to NIC driver for rx when
a page constructor API is invoked. Means NICs can allocate user buffers
from a page constructor. We add a hook in netif_receive_skb() function
to intercept the incoming packets, and notify the zero-copy device.
For write, the zero-copy deivce may allocates a new host skb and puts
payload on the skb_shinfo(skb)->frags, and copied the header to skb->data.
The request remains pending until the skb is transmitted by h/w.
We provide multiple submits and asynchronous notifiicaton to
vhost-net too.
Our goal is to improve the bandwidth and reduce the CPU usage.
Exact performance data will be provided later.
What we have not done yet:
Performance tuning
what we have done in v1:
polish the RCU usage
deal with write logging in asynchroush mode in vhost
add notifier block for mp device
rename page_ctor to mp_port in netdevice.h to make it looks generic
add mp_dev_change_flags() for mp device to change NIC state
add CONIFG_VHOST_MPASSTHRU to limit the usage when module is not load
a small fix for missing dev_put when fail
using dynamic minor instead of static minor number
a __KERNEL__ protect to mp_get_sock()
what we have done in v2:
remove most of the RCU usage, since the ctor pointer is only
changed by BIND/UNBIND ioctl, and during that time, NIC will be
stopped to get good cleanup(all outstanding requests are finished),
so the ctor pointer cannot be raced into wrong situation.
Remove the struct vhost_notifier with struct kiocb.
Let vhost-net backend to alloc/free the kiocb and transfer them
via sendmsg/recvmsg.
use get_user_pages_fast() and set_page_dirty_lock() when read.
Add some comments for netdev_mp_port_prep() and handle_mpassthru().
what we have done in v3:
the async write logging is rewritten
a drafted synchronous write function for qemu live migration
a limit for locked pages from get_user_pages_fast() to prevent Dos
by using RLIMIT_MEMLOCK
what we have done in v4:
add iocb completion callback from vhost-net to queue iocb in mp device
replace vq->receiver by mp_sock_data_ready()
remove stuff in mp device which access structures from vhost-net
modify skb_reserve() to ignore host NIC driver reserved space
rebase to the latest vhost tree
split large patches into small pieces, especially for net core part.
what we have done in v5:
address Arnd Bergmann's comments
-remove IFF_MPASSTHRU_EXCL flag in mp device
-Add CONFIG_COMPAT macro
-remove mp_release ops
move dev_is_mpassthru() as inline func
fix a bug in memory relinquish
Apply to current git (2.6.34-rc6) tree.
what we have done in v6:
move create_iocb() out of page_dtor which may happen in interrupt context
-This remove the potential issues which lock called in interrupt context
make the cache used by mp, vhost as static, and created/destoryed during
modules init/exit functions.
-This makes multiple mp guest created at the same time.
what we have done in v7:
some cleanup prepared to suppprt PS mode
what we have done in v8:
discarding the modifications to point skb->data to guest buffer directly.
Add code to modify driver to support napi_gro_frags() with Herbert's comments.
To support PS mode.
Add mergeable buffer support in mp device.
Add GSO/GRO support in mp deice.
Address comments from Eric Dumazet about cache line and rcu usage.
what we have done in v9:
v8 patch is based on a fix in dev_gro_receive().
But Herbert did not agree with the fix we have sent out.
And he suggest another fix. v9 is modified to base on that fix.
^ permalink raw reply
* [RFC PATCH v9 16/16] An example how to alloc user buffer based on napi_gro_frags() interface.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <1281086624-5765-16-git-send-email-xiaohui.xin@intel.com>
From: Xin Xiaohui <xiaohui.xin@intel.com>
This example is made on ixgbe driver which using napi_gro_frags().
It can get buffers from guest side directly using netdev_alloc_page()
and release guest buffers using netdev_free_page().
---
drivers/net/ixgbe/ixgbe_main.c | 25 +++++++++++++++++++++----
1 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index cfe6853..c563111 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -691,7 +691,14 @@ static inline void ixgbe_release_rx_desc(struct ixgbe_hw *hw,
static bool is_rx_buffer_mapped_as_page(struct ixgbe_rx_buffer *bi,
struct net_device *dev)
{
- return true;
+ return dev_is_mpassthru(dev);
+}
+
+static u32 get_page_skb_offset(struct net_device *dev)
+{
+ if (!dev_is_mpassthru(dev))
+ return 0;
+ return dev->mp_port->vnet_hlen;
}
/**
@@ -764,7 +771,8 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
adapter->alloc_rx_page_failed++;
goto no_buffers;
}
- bi->page_skb_offset = 0;
+ bi->page_skb_offset =
+ get_page_skb_offset(adapter->netdev);
bi->dma = pci_map_page(pdev, bi->page_skb,
bi->page_skb_offset,
(PAGE_SIZE / 2),
@@ -899,8 +907,10 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
len = le16_to_cpu(rx_desc->wb.upper.length);
}
- if (is_no_buffer(rx_buffer_info))
+ if (is_no_buffer(rx_buffer_info)) {
+ printk("no buffers\n");
break;
+ }
cleaned = true;
@@ -945,6 +955,12 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
rx_buffer_info->page_skb,
rx_buffer_info->page_skb_offset,
len);
+ if (dev_is_mpassthru(netdev) &&
+ netdev->mp_port->hash)
+ skb_shinfo(skb)->destructor_arg =
+ netdev->mp_port->hash(netdev,
+ rx_buffer_info->page_skb);
+
rx_buffer_info->page_skb = NULL;
skb->len += len;
skb->data_len += len;
@@ -962,7 +978,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
upper_len);
if ((rx_ring->rx_buf_len > (PAGE_SIZE / 2)) ||
- (page_count(rx_buffer_info->page) != 1))
+ (page_count(rx_buffer_info->page) != 1) ||
+ dev_is_mpassthru(netdev))
rx_buffer_info->page = NULL;
else
get_page(rx_buffer_info->page);
--
1.5.4.4
^ permalink raw reply related
* [RFC PATCH v9 13/16] Add a kconfig entry and make entry for mp device.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <1281086624-5765-12-git-send-email-xiaohui.xin@intel.com>
From: Xin Xiaohui <xiaohui.xin@intel.com>
Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
Reviewed-by: Jeff Dike <jdike@linux.intel.com>
---
drivers/vhost/Kconfig | 10 ++++++++++
drivers/vhost/Makefile | 2 ++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/drivers/vhost/Kconfig b/drivers/vhost/Kconfig
index e4e2fd1..a6b8cbf 100644
--- a/drivers/vhost/Kconfig
+++ b/drivers/vhost/Kconfig
@@ -9,3 +9,13 @@ config VHOST_NET
To compile this driver as a module, choose M here: the module will
be called vhost_net.
+config MEDIATE_PASSTHRU
+ tristate "mediate passthru network driver (EXPERIMENTAL)"
+ depends on VHOST_NET
+ ---help---
+ zerocopy network I/O support, we call it as mediate passthru to
+ be distiguish with hardare passthru.
+
+ To compile this driver as a module, choose M here: the module will
+ be called mpassthru.
+
diff --git a/drivers/vhost/Makefile b/drivers/vhost/Makefile
index 72dd020..c18b9fc 100644
--- a/drivers/vhost/Makefile
+++ b/drivers/vhost/Makefile
@@ -1,2 +1,4 @@
obj-$(CONFIG_VHOST_NET) += vhost_net.o
vhost_net-y := vhost.o net.o
+
+obj-$(CONFIG_MEDIATE_PASSTHRU) += mpassthru.o
--
1.5.4.4
^ permalink raw reply related
* [RFC PATCH v9 10/16] Add a hook to intercept external buffers from NIC driver.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <1281086624-5765-10-git-send-email-xiaohui.xin@intel.com>
From: Xin Xiaohui <xiaohui.xin@intel.com>
The hook is called in netif_receive_skb().
Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
Signed-off-by: Zhao Yu <yzhao81new@gmail.com>
Reviewed-by: Jeff Dike <jdike@linux.intel.com>
---
net/core/dev.c | 35 +++++++++++++++++++++++++++++++++++
1 files changed, 35 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 636f11b..4b379b1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2517,6 +2517,37 @@ err:
EXPORT_SYMBOL(netdev_mp_port_prep);
#endif
+#if defined(CONFIG_MEDIATE_PASSTHRU) || defined(CONFIG_MEDIATE_PASSTHRU_MODULE)
+/* Add a hook to intercept mediate passthru(zero-copy) packets,
+ * and insert it to the socket queue owned by mp_port specially.
+ */
+static inline struct sk_buff *handle_mpassthru(struct sk_buff *skb,
+ struct packet_type **pt_prev,
+ int *ret,
+ struct net_device *orig_dev)
+{
+ struct mpassthru_port *mp_port = NULL;
+ struct sock *sk = NULL;
+
+ if (!dev_is_mpassthru(skb->dev))
+ return skb;
+ mp_port = skb->dev->mp_port;
+
+ if (*pt_prev) {
+ *ret = deliver_skb(skb, *pt_prev, orig_dev);
+ *pt_prev = NULL;
+ }
+
+ sk = mp_port->sock->sk;
+ skb_queue_tail(&sk->sk_receive_queue, skb);
+ sk->sk_state_change(sk);
+
+ return NULL;
+}
+#else
+#define handle_mpassthru(skb, pt_prev, ret, orig_dev) (skb)
+#endif
+
/**
* netif_receive_skb - process receive buffer from network
* @skb: buffer to process
@@ -2598,6 +2629,10 @@ int netif_receive_skb(struct sk_buff *skb)
ncls:
#endif
+ /* To intercept mediate passthru(zero-copy) packets here */
+ skb = handle_mpassthru(skb, &pt_prev, &ret, orig_dev);
+ if (!skb)
+ goto out;
skb = handle_bridge(skb, &pt_prev, &ret, orig_dev);
if (!skb)
goto out;
--
1.5.4.4
^ permalink raw reply related
* [RFC PATCH v9 09/16] Don't do skb recycle, if device use external buffer.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <1281086624-5765-9-git-send-email-xiaohui.xin@intel.com>
From: Xin Xiaohui <xiaohui.xin@intel.com>
Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
Signed-off-by: Zhao Yu <yzhao81new@gmail.com>
Reviewed-by: Jeff Dike <jdike@linux.intel.com>
---
net/core/skbuff.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index bbf4707..9b156bb 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -565,6 +565,12 @@ int skb_recycle_check(struct sk_buff *skb, int skb_size)
if (skb_shared(skb) || skb_cloned(skb))
return 0;
+ /* if the device wants to do mediate passthru, the skb may
+ * get external buffer, so don't recycle
+ */
+ if (dev_is_mpassthru(skb->dev))
+ return 0;
+
skb_release_head_state(skb);
shinfo = skb_shinfo(skb);
atomic_set(&shinfo->dataref, 1);
--
1.5.4.4
^ permalink raw reply related
* [RFC PATCH v9 06/16] Use callback to deal with skb_release_data() specially.
From: xiaohui.xin @ 2010-08-06 9:23 UTC (permalink / raw)
To: netdev, kvm, linux-kernel, mst, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <1281086624-5765-6-git-send-email-xiaohui.xin@intel.com>
From: Xin Xiaohui <xiaohui.xin@intel.com>
If buffer is external, then use the callback to destruct
buffers.
Signed-off-by: Xin Xiaohui <xiaohui.xin@intel.com>
Signed-off-by: Zhao Yu <yzhao81new@gmail.com>
Reviewed-by: Jeff Dike <jdike@linux.intel.com>
---
include/linux/skbuff.h | 3 ++-
net/core/skbuff.c | 8 ++++++++
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 74af06c..ab29675 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -197,10 +197,11 @@ struct skb_shared_info {
union skb_shared_tx tx_flags;
struct sk_buff *frag_list;
struct skb_shared_hwtstamps hwtstamps;
- skb_frag_t frags[MAX_SKB_FRAGS];
/* Intermediate layers must ensure that destructor_arg
* remains valid until skb destructor */
void * destructor_arg;
+
+ skb_frag_t frags[MAX_SKB_FRAGS];
};
/* The structure is for a skb which pages may point to
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 93c4e06..117d82b 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -217,6 +217,7 @@ struct sk_buff *__alloc_skb(unsigned int size, gfp_t gfp_mask,
shinfo->gso_type = 0;
shinfo->ip6_frag_id = 0;
shinfo->tx_flags.flags = 0;
+ shinfo->destructor_arg = NULL;
skb_frag_list_init(skb);
memset(&shinfo->hwtstamps, 0, sizeof(shinfo->hwtstamps));
@@ -350,6 +351,13 @@ static void skb_release_data(struct sk_buff *skb)
if (skb_has_frags(skb))
skb_drop_fraglist(skb);
+ if (skb->dev && dev_is_mpassthru(skb->dev)) {
+ struct skb_ext_page *ext_page =
+ skb_shinfo(skb)->destructor_arg;
+ if (ext_page && ext_page->dtor)
+ ext_page->dtor(ext_page);
+ }
+
kfree(skb->head);
}
}
--
1.5.4.4
^ permalink raw reply related
* BUG: IPv6 stops working after a while, needs ip ne del command to reset
From: Thomas Habets @ 2010-08-06 8:25 UTC (permalink / raw)
To: netdev
IPv6 initially works, but when I leave it alone overnight I'm unable to
ping even my default gw.
Static global IPv6 addresses configured on both ends. No access lists on
either end.
Kernel version: 2.6.35 mainline (amd64) and 2.6.33.6.
Kernel config: http://pastebin.com/raw.php?i=Y6S8iKW7
Dist: Debian Lenny (5.0.5), nothing special to my knowledge.
I seem to have the same issue that Mikael Abrahamsson encountered with
Ubuntu kernels 2.6.26.3, 2.6.26-5-generic and 2.6.27-2-generic, and
mainline kernels 2.6.25, 2.6.26 and 2.6.27:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/263260
He got IPv6 running again without rebooting using "networking stop,
ifconfig eth0 down, networking start, kill dhclient", while I narrowed it
down to just deleting the ipv6 neighbor (ip ne del..., see below).
Rebooting also causes it to start working again.
It's very reproducible. I just leave it overnight and it breaks every
time.
I am willing and able to try patches at any time, the box is not in
production.
No iptables, no ip6tables. IP6tables support is not even compiled in.
NIC is "Broadcom Corporation NetXtreme BCM5715 Gigabit ethernet (rev a3)"
according to lspci.
Other end is a directly connected Cisco 7600 (routed port) that I have
access to, but it's in production use. IPv4 works perfectly over this same
port. Only lo and eth0 are UP.
Output when broken
------------------
$ uname -a
Linux XXXXX 2.6.35 #1 SMP Tue Aug 3 09:25:51 CEST 2010 x86_64
GNU/Linux
$ ip -6 a sh
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 16436
inet6 2a00:800:1000:64::1/128 scope global
valid_lft forever preferred_lft forever
inet6 ::1/128 scope host
valid_lft forever preferred_lft forever
2: eth0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qlen 1000
inet6 2a00:800:752:1::5c:2/112 scope global
valid_lft forever preferred_lft forever
inet6 fe80::224:81ff:fea3:4424/64 scope link
valid_lft forever preferred_lft forever
(I have tried removing 2a00:800:1000:64::1/128 from lo, same issue)
$ ip -6 r sh
2a00:800:752:1::5c:0/112 dev eth0 proto kernel metric 256 mtu 1500
advmss 14 hoplimit 4294967295 unreachable
2a00:800:1000:64::1 dev lo proto kernel metric 256 error -101 mtu 16436 advmss 16376 hoplimit
4294967295
fe80::/64 dev eth0 proto kernel metric 256 mtu 1500 advmss 1440
hoplimit 4294967295
default via 2a00:800:752:1::5c:1 dev eth0 metric 1024 mtu 1500 advmss
1440 hoplimit 4294967295
$ ping6 2a00:800:752:1::5c:1
PING 2a00:800:752:1::5c:1(2a00:800:752:1::5c:1) 56 data bytes
^C
--- 2a00:800:752:1::5c:1 ping statistics ---
22 packets transmitted, 0 received, 100% packet loss, time 21006ms
# Tcpdpump on the problem machine shows mostly the pings, but also
periodically some ND:
[...]
12:54:02.683672 00:24:81:a3:44:24 > 00:22:55:17:4b:80, ethertype IPv6
(0x86dd), length 118: 2a00:800:752:1::5c:2 > 2a00:800:752:1::5c:1: ICMP6,
echo request, seq 12, length 64
12:54:02.693669 00:24:81:a3:44:24 > 00:22:55:17:4b:80, ethertype IPv6
(0x86dd), length 86: fe80::224:81ff:fea3:4424 > 2a00:800:752:1::5c:1:
ICMP6, neighbor solicitation, who has 2a00:800:752:1::5c:1, length 32
12:54:02.693832 00:22:55:17:4b:80 > 00:24:81:a3:44:24, ethertype IPv6
(0x86dd), length 78: 2a00:800:752:1::5c:1 > fe80::224:81ff:fea3:4424:
ICMP6, neighbor advertisement, tgt is 2a00:800:752:1::5c:1, length 24
12:54:03.683672 00:24:81:a3:44:24 > 00:22:55:17:4b:80, ethertype IPv6
(0x86dd), length 118: 2a00:800:752:1::5c:2 > 2a00:800:752:1::5c:1: ICMP6,
echo request, seq 13, length 64
[...]
$ ip -6 ne
fe80::222:55ff:fe17:4b80 dev eth0 lladdr 00:22:55:17:4b:80 router STALE
2a00:800:752:1::5c:1 dev eth0 lladdr 00:22:55:17:4b:80 router STALE
Fixing the adjacency
--------------------
$ ping6 2a00:800:752:1::5c:1
PING 2a00:800:752:1::5c:1(2a00:800:752:1::5c:1) 56 data bytes
^C
--- 2a00:800:752:1::5c:1 ping statistics ---
51 packets transmitted, 0 received, 100% packet loss, time 50006ms
$ sudo ip ne del 2a00:800:752:1::5c:1 dev eth0
$ ping6 2a00:800:752:1::5c:1
PING 2a00:800:752:1::5c:1(2a00:800:752:1::5c:1) 56 data bytes
64 bytes from 2a00:800:752:1::5c:1: icmp_seq=1 ttl=64 time=31.9 ms
64 bytes from 2a00:800:752:1::5c:1: icmp_seq=2 ttl=64 time=0.212 ms
$ ip -6 ne
fe80::222:55ff:fe17:4b80 dev eth0 lladdr 00:22:55:17:4b:80 router
REACHABLE
2a00:800:752:1::5c:1 dev eth0 lladdr 00:22:55:17:4b:80 router REACHABLE
(Note that after a few minutes it goes back to STALE, but pinging still
works and brings back the state to REACHABLE, so it's not that it can't
get out of STALE once there, it seems).
---------
typedef struct me_s {
char name[] = { "Thomas Habets" };
char email[] = { "thomas@habets.pp.se" };
char kernel[] = { "Linux" };
char *pgpKey[] = { "http://www.habets.pp.se/pubkey.txt" };
char pgp[] = { "A8A3 D1DD 4AE0 8467 7FDE 0945 286A E90A AD48 E854" };
char coolcmd[] = { "echo '. ./_&. ./_'>_;. ./_" };
} me_t;
^ permalink raw reply
* Re: [PATCH 2/2] netfilter: xt_condition: change the value from boolean to u32
From: Jan Engelhardt @ 2010-08-06 8:52 UTC (permalink / raw)
To: Luciano Coelho
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <1281081606.1586.28.camel@powerslave>
On Friday 2010-08-06 10:00, Luciano Coelho wrote:
>> >+ buf[length - 1] = '\0';
>> >+
>> >+ if (strict_strtoull(buf, 0, &value) != 0)
>> >+ return -EINVAL;
>> >+
>> >+ if (value > (u32) value)
>> >+ return -EINVAL;
>>
>> Is it possible to use just strict_strtoul?
>
>Not easily. I found that there is a bug in strtoul (and strtoull for
>that matter) that causes the long to overflow if there are valid digits
>after the maximum possible digits for the base. For example if you try
>to strtoul 0xfffffffff (with 9 f's) the strtoul will overflow and come
>up with a bogus result.
I see. Strange that no one has adressed this yet - I mean, writing
a just-too-large value into a procfs/sysfs file and thus effectively
causing a bogus value to be actually written isn't quite so thrilling
as things go haywire.
>I can't easily truncate the string to avoid
>this problem, because with decimal or octal, the same valid value would
>take more spaces. I could do some magic here, checking whether it's a
>hex, dec or oct and truncate appropriately, but that would be very ugly.
>
>So the simplest way I came up with was to use strtoull and return
>-EINVAL if the value exceeds 32 bits. ;)
If I read strtoul(3) right, ERANGE is used for "out of range".
>> Since the condition value (cdmark) was thought of an nfmark-style thing,
>> would it perhaps make sense to model it after it
>>
>> return (var->value & ~info->mask) ^ info->value;
>>
>> Other opinions?
>
>I think it's nicer to have it as a normal equals here for now and then
>extend the match with more operations. We can later add, for example,
>an --and option to the condition match in order to do other kinds of
>binary operations. It would be more flexible this way because we could
>use several different types of comparisons, wouldn't it? And in the
>target we could have several different types of operations.
Indeed.
^ permalink raw reply
* [patch] isdn: gigaset: add missing unlock
From: Dan Carpenter @ 2010-08-06 8:23 UTC (permalink / raw)
To: Hansjoerg Lipp
Cc: Tilman Schmidt, Karsten Keil, David S. Miller, Andy Shevchenko,
gigaset307x-common, netdev, kernel-janitors
We should unlock here. This is the only place where we return from the
function with the lock held. The caller isn't expecting it.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c
index e5ea344..bcc174e 100644
--- a/drivers/isdn/gigaset/capi.c
+++ b/drivers/isdn/gigaset/capi.c
@@ -1052,6 +1052,7 @@ static inline void remove_appl_from_channel(struct bc_state *bcs,
do {
if (bcap->bcnext == ap) {
bcap->bcnext = bcap->bcnext->bcnext;
+ spin_unlock_irqrestore(&bcs->aplock, flags);
return;
}
bcap = bcap->bcnext;
^ permalink raw reply related
* [patch] isdn: gigaset: use after free
From: Dan Carpenter @ 2010-08-06 8:21 UTC (permalink / raw)
To: Hansjoerg Lipp
Cc: Tilman Schmidt, Karsten Keil, David S. Miller, Tejun Heo,
Joe Perches, gigaset307x-common, netdev, kernel-janitors
I moved the kfree(cb) below the dereferences.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c
index 0ded364..707d9c9 100644
--- a/drivers/isdn/gigaset/bas-gigaset.c
+++ b/drivers/isdn/gigaset/bas-gigaset.c
@@ -1914,11 +1914,13 @@ static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb)
* The next command will reopen the AT channel automatically.
*/
if (cb->len == 3 && !memcmp(cb->buf, "+++", 3)) {
- kfree(cb);
rc = req_submit(cs->bcs, HD_CLOSE_ATCHANNEL, 0, BAS_TIMEOUT);
if (cb->wake_tasklet)
tasklet_schedule(cb->wake_tasklet);
- return rc < 0 ? rc : cb->len;
+ if (!rc)
+ rc = cb->len;
+ kfree(cb);
+ return rc;
}
spin_lock_irqsave(&cs->cmdlock, flags);
^ permalink raw reply related
* Re: [PATCH 2/2] netfilter: xt_condition: change the value from boolean to u32
From: Luciano Coelho @ 2010-08-06 8:00 UTC (permalink / raw)
To: ext Jan Engelhardt
Cc: netfilter-devel@vger.kernel.org, netdev@vger.kernel.org,
kaber@trash.net, sameo@linux.intel.com
In-Reply-To: <alpine.LSU.2.01.1008051705320.1104@obet.zrqbmnf.qr>
On Thu, 2010-08-05 at 17:12 +0200, ext Jan Engelhardt wrote:
> On Thursday 2010-08-05 16:41, luciano.coelho@nokia.com wrote:
>
> > struct xt_condition_mtinfo {
> >- char name[31];
> >+ char name[27];
> > __u8 invert;
> >+ __u32 value;
>
> Please also bump the .revision field to 2 with this patch so that
> testing can always proceed without an ABI clash.
> (rev 2 would then remain over the course of the remaining patches
> you submit.)
> (rev 0 = ipt_condition/pom-ng; rev 1 = xt_condition from Xt-a)
Oh, did I forget that? I thought I had bumped it up in my previous patch
after your comment, but apparently I didn't. I'll fix it.
> >+ char buf[14];
>
> char buf[sizeof("4294967296")];
>
> seems more intuitive :-)
I'm using base 0 in the strtoull call, so I need at least 14 chars to
accommodate octals. In octal MAX_UINT takes 11 digits, plus the leading
0 that marks it as an octal, plus the optional + sign (not sure if this
one makes sense, though), plus the null-terminator (or actually the \n
when writing to procfs) leads to 14. But I'm not sure whether:
char buf[sizeof("+037777777777")];
Would be very intuitive? Surely I don't like the magic number, so I
should probably change it anyway.
> >+ unsigned long long value;
> >+
> >+ if (length == 0)
> >+ return 0;
> >+
> >+ if (length > sizeof(buf))
> >+ return -EINVAL;
> >+
> >+ if (copy_from_user(buf, input, length) != 0)
> >+ return -EFAULT;
> >+
> >+ buf[length - 1] = '\0';
> >+
> >+ if (strict_strtoull(buf, 0, &value) != 0)
> >+ return -EINVAL;
> >+
> >+ if (value > (u32) value)
> >+ return -EINVAL;
>
> Is it possible to use just strict_strtoul?
Not easily. I found that there is a bug in strtoul (and strtoull for
that matter) that causes the long to overflow if there are valid digits
after the maximum possible digits for the base. For example if you try
to strtoul 0xfffffffff (with 9 f's) the strtoul will overflow and come
up with a bogus result. I can't easily truncate the string to avoid
this problem, because with decimal or octal, the same valid value would
take more spaces. I could do some magic here, checking whether it's a
hex, dec or oct and truncate appropriately, but that would be very ugly.
So the simplest way I came up with was to use strtoull and return
-EINVAL if the value exceeds 32 bits. ;)
> >- return var->enabled ^ info->invert;
> >+ return (var->value == info->value) ^ info->invert;
>
> Since the condition value (cdmark) was thought of an nfmark-style thing,
> would it perhaps make sense to model it after it
>
> return (var->value & ~info->mask) ^ info->value;
>
> Other opinions?
I think it's nicer to have it as a normal equals here for now and then
extend the match with more operations. We can later add, for example,
an --and option to the condition match in order to do other kinds of
binary operations. It would be more flexible this way because we could
use several different types of comparisons, wouldn't it? And in the
target we could have several different types of operations.
--
Cheers,
Luca.
^ permalink raw reply
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 7:37 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev, David S. Miller
In-Reply-To: <AANLkTi=_-ZZ8nyFLiBKSWNTcM_SkwLe2-ZL17zBQb0i4@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 882 bytes --]
On Friday, August 06, 2010 02:43:46 Changli Gao wrote:
> On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger wrote:
> > If the SMSC phy is not built as a module, we get the warning:
> > drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not
> > used
> >
> > So mark the structure as maybe unused to avoid this.
> >
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> > module_init(smsc_init);
> > module_exit(smsc_exit);
> >
> > -static struct mdio_device_id smsc_tbl[] = {
> > +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> > { 0x0007c0a0, 0xfffffff0 },
> > { 0x0007c0b0, 0xfffffff0 },
> > { 0x0007c0c0, 0xfffffff0 },
>
> How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these
> lines?
imo, the less ifdefs the better
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 7:37 UTC (permalink / raw)
To: Changli Gao; +Cc: netdev, David S. Miller
In-Reply-To: <AANLkTi=_-ZZ8nyFLiBKSWNTcM_SkwLe2-ZL17zBQb0i4@mail.gmail.com>
[-- Attachment #1: Type: Text/Plain, Size: 1125 bytes --]
On Friday, August 06, 2010 02:43:46 Changli Gao wrote:
> On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> > If the SMSC phy is not built as a module, we get the warning:
> > drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not
> > used
> >
> > So mark the structure as maybe unused to avoid this.
> >
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > ---
> > drivers/net/phy/smsc.c | 2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> > index 78fa988..11944ef 100644
> > --- a/drivers/net/phy/smsc.c
> > +++ b/drivers/net/phy/smsc.c
> > @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> > module_init(smsc_init);
> > module_exit(smsc_exit);
> >
> > -static struct mdio_device_id smsc_tbl[] = {
> > +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> > { 0x0007c0a0, 0xfffffff0 },
> > { 0x0007c0b0, 0xfffffff0 },
> > { 0x0007c0c0, 0xfffffff0 },
>
> How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these
> lines?
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH 32/42] drivers/net/bnx2x: Adjust confusing if indentation
From: Dan Carpenter @ 2010-08-06 7:07 UTC (permalink / raw)
To: Julia Lawall; +Cc: Eilon Greenstein, netdev, linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008052226210.31692@ask.diku.dk>
On Thu, Aug 05, 2010 at 10:26:38PM +0200, Julia Lawall wrote:
> ---
> This patch doesn't change the semantics of the code. But it might not be
> what is intended.
>
I think there may have been some if statements which were removed before
the code was merged into the kernel? Here is another one from that same
function. Someone can roll this in with your patch.
regards,
dan carpenter
diff --git a/drivers/net/bnx2x/bnx2x_link.c b/drivers/net/bnx2x/bnx2x_link.c
index 0fa0991..ca0e435 100644
--- a/drivers/net/bnx2x/bnx2x_link.c
+++ b/drivers/net/bnx2x/bnx2x_link.c
@@ -1242,11 +1242,11 @@ static void bnx2x_set_autoneg(struct link_params *params,
MDIO_CL73_USERB0_CL73_BAM_CTRL1_BAM_NP_AFTER_BP_EN);
/* Advertise CL73 link speeds */
- CL45_RD_OVER_CL22(bp, params->port,
- params->phy_addr,
- MDIO_REG_BANK_CL73_IEEEB1,
- MDIO_CL73_IEEEB1_AN_ADV2,
- ®_val);
+ CL45_RD_OVER_CL22(bp, params->port,
+ params->phy_addr,
+ MDIO_REG_BANK_CL73_IEEEB1,
+ MDIO_CL73_IEEEB1_AN_ADV2,
+ ®_val);
if (params->speed_cap_mask &
PORT_HW_CFG_SPEED_CAPABILITY_D0_10G)
reg_val |= MDIO_CL73_IEEEB1_AN_ADV2_ADVR_10G_KX4;
^ permalink raw reply related
* Re: [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Changli Gao @ 2010-08-06 6:43 UTC (permalink / raw)
To: Mike Frysinger; +Cc: netdev, David S. Miller
In-Reply-To: <1281076034-17390-1-git-send-email-vapier@gentoo.org>
On Fri, Aug 6, 2010 at 2:27 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> If the SMSC phy is not built as a module, we get the warning:
> drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not used
>
> So mark the structure as maybe unused to avoid this.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> ---
> drivers/net/phy/smsc.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
> index 78fa988..11944ef 100644
> --- a/drivers/net/phy/smsc.c
> +++ b/drivers/net/phy/smsc.c
> @@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
> module_init(smsc_init);
> module_exit(smsc_exit);
>
> -static struct mdio_device_id smsc_tbl[] = {
> +static __maybe_unused struct mdio_device_id smsc_tbl[] = {
> { 0x0007c0a0, 0xfffffff0 },
> { 0x0007c0b0, 0xfffffff0 },
> { 0x0007c0c0, 0xfffffff0 },
How about adding a #ifdef CONFIG_SMSC_PHY_MODULE clause around these lines?
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* [PATCH] phy: SMSC: fix unused warnings when built into kernel
From: Mike Frysinger @ 2010-08-06 6:27 UTC (permalink / raw)
To: netdev, David S. Miller
If the SMSC phy is not built as a module, we get the warning:
drivers/net/phy/smsc.c:257: warning: 'smsc_tbl' defined but not used
So mark the structure as maybe unused to avoid this.
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
drivers/net/phy/smsc.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/phy/smsc.c b/drivers/net/phy/smsc.c
index 78fa988..11944ef 100644
--- a/drivers/net/phy/smsc.c
+++ b/drivers/net/phy/smsc.c
@@ -254,7 +254,7 @@ MODULE_LICENSE("GPL");
module_init(smsc_init);
module_exit(smsc_exit);
-static struct mdio_device_id smsc_tbl[] = {
+static __maybe_unused struct mdio_device_id smsc_tbl[] = {
{ 0x0007c0a0, 0xfffffff0 },
{ 0x0007c0b0, 0xfffffff0 },
{ 0x0007c0c0, 0xfffffff0 },
--
1.7.2
^ permalink raw reply related
* Re: e1000e crashes with 2.6.34.x and ThinkPad T60
From: Chuck Ebbert @ 2010-08-06 5:46 UTC (permalink / raw)
To: Marc Haber
Cc: e1000-devel@lists.sourceforge.net,
Linux Kernel Network Developers, Allan, Bruce W,
Linux Kernel Developers
In-Reply-To: <20100730125614.GB21444@torres.zugschlus.de>
On Fri, 30 Jul 2010 14:56:14 +0200
Marc Haber <mh+linux-kernel@zugschlus.de> wrote:
> Capabilities: <access denied>
That's not very useful.
The pcie capabilities are completely missing.
------------------------------------------------------------------------------
This SF.net email is sponsored by
Make an app they can't live without
Enter the BlackBerry Developer Challenge
http://p.sf.net/sfu/RIM-dev2dev
_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit http://communities.intel.com/community/wired
^ permalink raw reply
* Re: [GIT] Networking
From: David Miller @ 2010-08-06 5:19 UTC (permalink / raw)
To: yinghai; +Cc: torvalds, akpm, netdev, linux-kernel
In-Reply-To: <4C5B965F.4020109@kernel.org>
We know about this FCOE build issue, it's config specific, and the
Intel folks have a patch coming to me tonight that will fix this up.
Thanks.
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 4:58 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTi=3dVWKtgL2A=4xm_1VSk64--3Zk_fQBYUdAAn+@mail.gmail.com>
On 08/05/2010 07:15 PM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> CC drivers/net/ixgbe/ixgbe_main.o
>> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
>> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
>
> Hmm. You must have some other version of that file than I do. Line
> 6159 is in ixgbe_init_module() in my source tree.
>
http://patchwork.ozlabs.org/patch/60777/
Yinghai
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 4:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTi=3dVWKtgL2A=4xm_1VSk64--3Zk_fQBYUdAAn+@mail.gmail.com>
On 08/05/2010 07:15 PM, Linus Torvalds wrote:
> On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> CC drivers/net/ixgbe/ixgbe_main.o
>> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
>> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
>
> Hmm. You must have some other version of that file than I do. Line
> 6159 is in ixgbe_init_module() in my source tree.
>
> Linus
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=drivers/net/ixgbe/ixgbe_main.c;h=7d6a415bcf885633999e10ed795df49876d6c47e;hb=HEAD
6146 static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
6147 {
6148 struct ixgbe_adapter *adapter = netdev_priv(dev);
6149 int txq = smp_processor_id();
6150
6151 #ifdef IXGBE_FCOE
6152 if ((skb->protocol == htons(ETH_P_FCOE)) ||
6153 (skb->protocol == htons(ETH_P_FIP))) {
6154 if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
6155 txq &= (adapter->ring_feature[RING_F_FCOE].indices - 1);
6156 txq += adapter->ring_feature[RING_F_FCOE].mask;
6157 return txq;
6158 } else if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
6159 txq = adapter->fcoe.up;
6160 return txq;
6161 }
6162 }
6163 #endif
^ permalink raw reply
* Re: [GIT] Networking
From: Linus Torvalds @ 2010-08-06 2:15 UTC (permalink / raw)
To: Yinghai Lu; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTinU4+tcm2O_BVBFkHSdnx7JEPtM4sw_s--qdUKC@mail.gmail.com>
On Thu, Aug 5, 2010 at 7:04 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>
> CC drivers/net/ixgbe/ixgbe_main.o
> drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
> drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no member named ‘up’
Hmm. You must have some other version of that file than I do. Line
6159 is in ixgbe_init_module() in my source tree.
Linus
^ permalink raw reply
* Re: [GIT] Networking
From: Yinghai Lu @ 2010-08-06 2:04 UTC (permalink / raw)
To: Linus Torvalds; +Cc: David Miller, akpm, netdev, linux-kernel
In-Reply-To: <AANLkTimxv1y3QJ7s+ghi1FFCcwnUVWT1n6dvT-NNN=Oo@mail.gmail.com>
On Wed, Aug 4, 2010 at 12:06 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Tue, Aug 3, 2010 at 8:38 PM, David Miller <davem@davemloft.net> wrote:
>>
>> Another release, another merge window, another set of networking
>> changes to merge :-)
>
> Ok, merged. But you should double-check my merge resolution fixes,
CC drivers/net/ixgbe/ixgbe_main.o
drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_select_queue’:
drivers/net/ixgbe/ixgbe_main.c:6159: error: ‘struct ixgbe_fcoe’ has no
member named ‘up’
drivers/net/ixgbe/ixgbe_main.c: In function ‘ixgbe_xmit_frame’:
drivers/net/ixgbe/ixgbe_main.c:6221: error: ‘struct ixgbe_fcoe’ has no
member named ‘up’
make[1]: *** [drivers/net/ixgbe/ixgbe_main.o] Error 1
make: *** [drivers/net/ixgbe/] Error 2
YH
^ permalink raw reply
* Re: [rfc 10/13] [RFC 10/13] IPVS: management of persistence engine modules
From: Simon Horman @ 2010-08-06 1:48 UTC (permalink / raw)
To: Stephen Hemminger
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <20100805092920.7573c96b@nehalam>
On Thu, Aug 05, 2010 at 09:29:20AM -0700, Stephen Hemminger wrote:
> On Thu, 05 Aug 2010 20:48:05 +0900
> Simon Horman <horms@verge.net.au> wrote:
>
> > +/* lock for service table */
> > +static DEFINE_RWLOCK(__ip_vs_pe_lock);
>
> It is already static so why the __?
> Reader/writer locks are slower than spinlocks. Either use
> a spinlock, or RCU (if possible)
No good reason, other than I copied the code from elsewhere.
I'll fix both this and the code that I copied.
> > +/* Bind a service with a pe */
> > +void ip_vs_bind_pe(struct ip_vs_service *svc, struct ip_vs_pe *pe)
> > +{
> > + svc->pe = pe;
> > +}
> > +
> > +/* Unbind a service from its pe */
> > +void ip_vs_unbind_pe(struct ip_vs_service *svc)
> > +{
> > + svc->pe = NULL;
> > +}
>
> What does having these wrappers buy?
Again, I copied the code from elsewhere, where the wrappers did more. As
it happens, I think that these do make some sense as they are called along
side other bind and unbind calls. But if you have a strong aversion to them
then I am happy to remove these wrappers.
^ permalink raw reply
* Re: [rfc 13/13] [RFC 13/13] IPVS: sip persistence engine
From: Simon Horman @ 2010-08-06 1:46 UTC (permalink / raw)
To: Jan Engelhardt
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <alpine.LSU.2.01.1008051447100.13201@obet.zrqbmnf.qr>
On Thu, Aug 05, 2010 at 02:50:11PM +0200, Jan Engelhardt wrote:
> On Thursday 2010-08-05 13:48, Simon Horman wrote:
> >+
> >+static const char *ip_vs_dbg_callid(char *buf, size_t buf_len,
> >+ const char *callid, size_t callid_len,
> >+ int *idx)
> >+{
> >+ size_t len = min(min(callid_len, (size_t)64), buf_len - *idx - 1);
> >+ memcpy(buf + *idx, callid, len);
> >+ *(buf + *idx + len) = '\0';
>
> Nothing particular wrong, though I just noticed that I would have
> personally, instinctively written
>
> buf[*idx+len] = '\0';
>
> instead. Sure you have your reasons ;-)
Actually, I had a bit of trouble writing that code clearly
(though I'm not sure why, perhaps it was late in the day).
I think your suggestion looks good.
> >+ /* RFC 2543 allows lines to be terminated with CR, LF or CRLF,
> >+ * RFC 3261 allows only CRLF, we support both. */
> >+ if (*(dptr + *matchoff + *matchlen) != '\r' &&
> >+ *(dptr + *matchoff + *matchlen) != '\n')
> >+ return -EINVAL;
^ permalink raw reply
* Re: [rfc 09/13] [RFC 09/13] IPVS: Add persistence engine data to /proc/net/ip_vs_conn
From: Simon Horman @ 2010-08-06 1:45 UTC (permalink / raw)
To: Jan Engelhardt
Cc: lvs-devel, netdev, netfilter, netfilter-devel, Wensong Zhang,
Julian Anastasov, Patrick McHardy
In-Reply-To: <alpine.LSU.2.01.1008051446060.13201@obet.zrqbmnf.qr>
On Thu, Aug 05, 2010 at 02:46:22PM +0200, Jan Engelhardt wrote:
> On Thursday 2010-08-05 13:48, Simon Horman wrote:
>
> >This shouldn't break compatibility with userspace as the new data
> >is at the end of the line.
> >
> >I have confirmed that this doesn't break ipvsadm, the main (only?)
> >user-space user of this data.
>
> Wouldn't it be best if this be done over Netlink?
I'm happy to add code to do that over netlink,
though it does seem to be overkill to me.
^ 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