* Re: [Bug #15042] socket(PF_INET6 hangs when ipv6 not yet initialized
From: Marc Haber @ 2010-02-14 6:20 UTC (permalink / raw)
To: Américo Wang
Cc: Rafael J. Wysocki, Linux Kernel Mailing List, Kernel Testers List,
Linux Kernel Network Developers
In-Reply-To: <2375c9f91001270110t2a8d08b8n4578df310a6129@mail.gmail.com>
Hi,
sorry for the late reply.
On Wed, Jan 27, 2010 at 05:10:44PM +0800, Américo Wang wrote:
> On Mon, Jan 25, 2010 at 6:23 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > This message has been generated automatically as a part of a report
> > of regressions introduced between 2.6.31 and 2.6.32.
> >
> > The following bug entry is on the current list of known regressions
> > introduced between 2.6.31 and 2.6.32. Please verify if it still should
> > be listed and let me know (either way).
> >
> >
> > Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=15042
> > Subject : socket(PF_INET6 hangs when ipv6 not yet initialized
> > Submitter : Marc Haber <mh+linux-kernel@zugschlus.de>
> > Date : 2010-01-10 14:28 (15 days old)
> > References : http://marc.info/?l=linux-kernel&m=126313553029280&w=4
> >
>
> (Adding net-dev into Cc)
>
> Hi, Marc,
>
> Sorry, I don't really understand your description of the problem, your mean
> your ssh hangs on socket PF_INET6 for 30 seconds?
No, it hangs on socket PF_INET6 indefinetely (max time I waited was
like ten minutes), ignoring SIGTERM while hanging. This does only
happen if the sshd is started within seconds of the ipv6 module being
loaded (or the Interface being brought up), so I guess that the kernel
needs to be in a state where it already thinks that it has IPv6 but
hasn't completed autoconfiguration yet.
To obtain an strace, I had to use timeout --signal=9 30. The
virtualbox process this happens inside takes all host CPU while the
ssh process hangs.
> Have you enabled CONFIG_DETECT_SOFTLOCKUP for your kernel?
yes, that's enabled.
Greetings
Marc
--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Mannheim, Germany | lose things." Winona Ryder | Fon: *49 621 72739834
Nordisch by Nature | How to make an American Quilt | Fax: *49 3221 2323190
Bitte beachten Sie, daß dem [m.E. grundgesetzwidrigen] Gesetz zur
Vorratsdatenspeicherung zufolge, seit dem 1. Januar 2008 jeglicher
elektronische Kontakt (E-Mail, Telefongespräche, SMS, Internet-
Telefonie, Mobilfunk, Fax) mit mir oder anderen Nutzern verdachts-
unabhängig für den automatisierten geheimen Zugriff durch Strafver-
folgungs- u. Polizeivollzugsbehörden, die Bundesanstalt für Finanz-
dienstleistungsaufsicht, Zollkriminal- und Zollfahndungsämter,die
Zollverwaltung zur Schwarzarbeitsbekämpfung, Notrufabfragestellen,
Verfassungsschutzbehörden, den Militärischen Abschirmdienst, Bundes-
nachrichtendienst sowie 52 Staaten wie beispielsweise Aserbeidschan
oder die USA sechs Monate lang gespeichert wird, einschließlich der
Kommunikation mit Berufsgeheimnisträgern wie Ärzten, Journalisten und
Anwälten. Mehr Infos zur totalen Protokollierung Ihrer Kommunikations-
daten auf www.vorratsdatenspeicherung.de. (leicht verändert übernommen
kopiert von www.lawblog.de)
^ permalink raw reply
* [PATCH v5 3/7] tcp: harmonize tcp_vx_rcv header length assumptions
From: William Allen Simpson @ 2010-02-14 6:16 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller, Andi Kleen
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
Harmonize tcp_v4_rcv() and tcp_v6_rcv() -- better document tcp doff
and header length assumptions, and carefully compare implementations.
Reduces multiply/shifts, marginally improving speed.
Removes redundant tcp header length checks before checksumming.
Instead, assumes (and documents) that any backlog processing and
transform policies will carefully preserve the header, and will
ensure the socket buffer length remains >= the header size.
Stand-alone patch, originally developed for TCPCT.
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Andi Kleen <andi@firstfloor.org>
---
include/net/xfrm.h | 7 ++++++
net/ipv4/tcp_ipv4.c | 45 +++++++++++++++++++++-----------------
net/ipv6/tcp_ipv6.c | 59 ++++++++++++++++++++++++++++----------------------
3 files changed, 65 insertions(+), 46 deletions(-)
[-- Attachment #2: len_th+2c5+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 8037 bytes --]
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 60c2770..81492a1 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -975,6 +975,13 @@ xfrm_state_addr_cmp(struct xfrm_tmpl *tmpl, struct xfrm_state *x, unsigned short
}
#ifdef CONFIG_XFRM
+/*
+ * For transport, the policy is checked before the presumed more expensive
+ * checksum. The transport header has already been checked for size, and is
+ * guaranteed to be contiguous. These policies must not alter the header or
+ * its position in the buffer, and should not shorten the buffer length
+ * without ensuring the length remains >= the header size.
+ */
extern int __xfrm_policy_check(struct sock *, int dir, struct sk_buff *skb, unsigned short family);
static inline int __xfrm_policy_check2(struct sock *sk, int dir,
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 65b8ebf..0a76e41 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1559,7 +1559,8 @@ int tcp_v4_do_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
- if (skb->len < tcp_hdrlen(skb) || tcp_checksum_complete(skb))
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb))
goto csum_err;
if (sk->sk_state == TCP_LISTEN) {
@@ -1601,14 +1602,13 @@ csum_err:
}
/*
- * From tcp_input.c
+ * Called by ip_input.c: ip_local_deliver_finish()
*/
-
int tcp_v4_rcv(struct sk_buff *skb)
{
- const struct iphdr *iph;
struct tcphdr *th;
struct sock *sk;
+ int tcp_header_len;
int ret;
struct net *net = dev_net(skb->dev);
@@ -1618,31 +1618,33 @@ int tcp_v4_rcv(struct sk_buff *skb)
/* Count it even if it's bad */
TCP_INC_STATS_BH(net, TCP_MIB_INSEGS);
+ /* Check too short header */
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
- th = tcp_hdr(skb);
-
- if (th->doff < sizeof(struct tcphdr) / 4)
+ /* Check bad doff, compare doff directly to constant value */
+ tcp_header_len = tcp_hdr(skb)->doff;
+ if (tcp_header_len < (sizeof(struct tcphdr) / 4))
goto bad_packet;
- if (!pskb_may_pull(skb, th->doff * 4))
+
+ /* Check too short header and options */
+ tcp_header_len *= 4;
+ if (!pskb_may_pull(skb, tcp_header_len))
goto discard_it;
- /* An explanation is required here, I think.
- * Packet length and doff are validated by header prediction,
- * provided case of th->doff==0 is eliminated.
- * So, we defer the checks. */
+ /* Packet length and doff are validated by header prediction,
+ * provided case of th->doff == 0 is eliminated (above).
+ */
if (!skb_csum_unnecessary(skb) && tcp_v4_checksum_init(skb))
goto bad_packet;
th = tcp_hdr(skb);
- iph = ip_hdr(skb);
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff * 4);
+ skb->len - tcp_header_len);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
- TCP_SKB_CB(skb)->flags = iph->tos;
+ TCP_SKB_CB(skb)->flags = ip_hdr(skb)->tos;
TCP_SKB_CB(skb)->sacked = 0;
sk = __inet_lookup_skb(&tcp_hashinfo, skb, th->source, th->dest);
@@ -1682,14 +1684,14 @@ process:
bh_unlock_sock(sk);
sock_put(sk);
-
return ret;
no_tcp_socket:
if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
- if (skb->len < (th->doff << 2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
bad_packet:
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
} else {
@@ -1711,18 +1713,21 @@ do_time_wait:
goto discard_it;
}
- if (skb->len < (th->doff << 2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
inet_twsk_put(inet_twsk(sk));
goto discard_it;
}
+
switch (tcp_timewait_state_process(inet_twsk(sk), skb, th)) {
case TCP_TW_SYN: {
struct sock *sk2 = inet_lookup_listener(dev_net(skb->dev),
&tcp_hashinfo,
- iph->daddr, th->dest,
+ ip_hdr(skb)->daddr,
+ th->dest,
inet_iif(skb));
- if (sk2) {
+ if (sk2 != NULL) {
inet_twsk_deschedule(inet_twsk(sk), &tcp_death_row);
inet_twsk_put(inet_twsk(sk));
sk = sk2;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index febfd59..b76939a 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1594,7 +1594,8 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
return 0;
}
- if (skb->len < tcp_hdrlen(skb) || tcp_checksum_complete(skb))
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb))
goto csum_err;
if (sk->sk_state == TCP_LISTEN) {
@@ -1664,38 +1665,47 @@ ipv6_pktoptions:
return 0;
}
+/*
+ * Called by ip6_input.c: ip6_input_finish()
+ */
static int tcp_v6_rcv(struct sk_buff *skb)
{
struct tcphdr *th;
struct sock *sk;
+ int tcp_header_len;
int ret;
struct net *net = dev_net(skb->dev);
if (skb->pkt_type != PACKET_HOST)
goto discard_it;
- /*
- * Count it even if it's bad.
- */
+ /* Count it even if it's bad */
TCP_INC_STATS_BH(net, TCP_MIB_INSEGS);
+ /* Check too short header */
if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
goto discard_it;
- th = tcp_hdr(skb);
-
- if (th->doff < sizeof(struct tcphdr)/4)
+ /* Check bad doff, compare doff directly to constant value */
+ tcp_header_len = tcp_hdr(skb)->doff;
+ if (tcp_header_len < (sizeof(struct tcphdr) / 4))
goto bad_packet;
- if (!pskb_may_pull(skb, th->doff*4))
+
+ /* Check too short header and options */
+ tcp_header_len *= 4;
+ if (!pskb_may_pull(skb, tcp_header_len))
goto discard_it;
+ /* Packet length and doff are validated by header prediction,
+ * provided case of th->doff == 0 is eliminated (above).
+ */
if (!skb_csum_unnecessary(skb) && tcp_v6_checksum_init(skb))
goto bad_packet;
th = tcp_hdr(skb);
TCP_SKB_CB(skb)->seq = ntohl(th->seq);
TCP_SKB_CB(skb)->end_seq = (TCP_SKB_CB(skb)->seq + th->syn + th->fin +
- skb->len - th->doff*4);
+ skb->len - tcp_header_len);
TCP_SKB_CB(skb)->ack_seq = ntohl(th->ack_seq);
TCP_SKB_CB(skb)->when = 0;
TCP_SKB_CB(skb)->flags = ipv6_get_dsfield(ipv6_hdr(skb));
@@ -1711,6 +1721,7 @@ process:
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
+ /* nf_reset(skb); in ip6_input.c ip6_input_finish() */
if (sk_filter(sk, skb))
goto discard_and_relse;
@@ -1743,7 +1754,8 @@ no_tcp_socket:
if (!xfrm6_policy_check(NULL, XFRM_POLICY_IN, skb))
goto discard_it;
- if (skb->len < (th->doff<<2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
bad_packet:
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
} else {
@@ -1751,11 +1763,7 @@ bad_packet:
}
discard_it:
-
- /*
- * Discard frame
- */
-
+ /* Discard frame. */
kfree_skb(skb);
return 0;
@@ -1769,24 +1777,23 @@ do_time_wait:
goto discard_it;
}
- if (skb->len < (th->doff<<2) || tcp_checksum_complete(skb)) {
+ /* Assumes header and options unchanged since checksum_init() */
+ if (tcp_checksum_complete(skb)) {
TCP_INC_STATS_BH(net, TCP_MIB_INERRS);
inet_twsk_put(inet_twsk(sk));
goto discard_it;
}
switch (tcp_timewait_state_process(inet_twsk(sk), skb, th)) {
- case TCP_TW_SYN:
- {
- struct sock *sk2;
-
- sk2 = inet6_lookup_listener(dev_net(skb->dev), &tcp_hashinfo,
- &ipv6_hdr(skb)->daddr,
- ntohs(th->dest), inet6_iif(skb));
+ case TCP_TW_SYN: {
+ struct sock *sk2 = inet6_lookup_listener(dev_net(skb->dev),
+ &tcp_hashinfo,
+ &ipv6_hdr(skb)->daddr,
+ ntohs(th->dest),
+ inet6_iif(skb));
if (sk2 != NULL) {
- struct inet_timewait_sock *tw = inet_twsk(sk);
- inet_twsk_deschedule(tw, &tcp_death_row);
- inet_twsk_put(tw);
+ inet_twsk_deschedule(inet_twsk(sk), &tcp_death_row);
+ inet_twsk_put(inet_twsk(sk));
sk = sk2;
goto process;
}
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 2/7] net: remove old tcp_optlen function
From: William Allen Simpson @ 2010-02-14 6:09 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller, Michael Chan
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]
The tcp_optlen() function returns a potential *negative* unsigned.
In the only two existing files using the old tcp_optlen() function,
clean up confusing and inconsistent mixing of both byte and word
offsets, and other coding style issues. Document assumptions.
Quoth David Miller:
This is transmit, and the packets can only come from the Linux
TCP stack, not some external entity.
You're being way too anal here, and adding these checks to
drivers would be just a lot of rediculious bloat. [sic]
Therefore, there are *no* checks for bad TCP and IP header sizes, nor
any semantic changes. The drivers should function exactly as existing.
No response from testers in 17+ weeks.
Requires:
net: tcp_header_len_th and tcp_option_len_th
Signed-off-by: William.Allen.Simpson@gmail.com
CC: Michael Chan <mchan@broadcom.com>
---
drivers/net/bnx2.c | 29 +++++++++++++-----------
drivers/net/tg3.c | 60 +++++++++++++++++++++++---------------------------
include/linux/tcp.h | 5 ----
3 files changed, 44 insertions(+), 50 deletions(-)
[-- Attachment #2: len_th+2b3+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 6936 bytes --]
diff --git a/drivers/net/bnx2.c b/drivers/net/bnx2.c
index 65df1de..45452c5 100644
--- a/drivers/net/bnx2.c
+++ b/drivers/net/bnx2.c
@@ -6352,6 +6352,8 @@ bnx2_vlan_rx_register(struct net_device *dev, struct vlan_group *vlgrp)
/* Called with netif_tx_lock.
* bnx2_tx_int() runs without netif_tx_lock unless it needs to call
* netif_wake_queue().
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t
bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
@@ -6396,19 +6398,19 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
(TX_BD_FLAGS_VLAN_TAG | (vlan_tx_tag_get(skb) << 16));
}
#endif
- if ((mss = skb_shinfo(skb)->gso_size)) {
- u32 tcp_opt_len;
- struct iphdr *iph;
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
+ struct tcphdr *th = tcp_hdr(skb);
+ int tcp_opt_words = th->doff - (sizeof(*th) >> 2);
+ /* assumes positive tcp_opt_words without checking */
vlan_tag_flags |= TX_BD_FLAGS_SW_LSO;
- tcp_opt_len = tcp_optlen(skb);
-
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6) {
u32 tcp_off = skb_transport_offset(skb) -
sizeof(struct ipv6hdr) - ETH_HLEN;
- vlan_tag_flags |= ((tcp_opt_len >> 2) << 8) |
+ vlan_tag_flags |= (tcp_opt_words << 8) |
TX_BD_FLAGS_SW_FLAGS;
if (likely(tcp_off == 0))
vlan_tag_flags &= ~TX_BD_FLAGS_TCP6_OFF0_MSK;
@@ -6421,14 +6423,15 @@ bnx2_start_xmit(struct sk_buff *skb, struct net_device *dev)
mss |= (tcp_off & 0xc) << TX_BD_TCP6_OFF2_SHL;
}
} else {
- iph = ip_hdr(skb);
- if (tcp_opt_len || (iph->ihl > 5)) {
- vlan_tag_flags |= ((iph->ihl - 5) +
- (tcp_opt_len >> 2)) << 8;
- }
+ struct iphdr *iph = ip_hdr(skb);
+ int ip_opt_words = iph->ihl - (sizeof(*iph) >> 2);
+ /* assumes positive ip_opt_words without checking */
+ int opt_words = ip_opt_words + tcp_opt_words;
+
+ if (opt_words > 0)
+ vlan_tag_flags |= opt_words << 8;
}
- } else
- mss = 0;
+ }
mapping = pci_map_single(bp->pdev, skb->data, len, PCI_DMA_TODEVICE);
if (pci_dma_mapping_error(bp->pdev, mapping)) {
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c
index 7f82b02..c20c800 100644
--- a/drivers/net/tg3.c
+++ b/drivers/net/tg3.c
@@ -5426,6 +5426,8 @@ static void tg3_set_txd(struct tg3_napi *tnapi, int entry,
/* hard_start_xmit for devices that don't have any bugs and
* support TG3_FLG2_HW_TSO_2 and TG3_FLG2_HW_TSO_3 only.
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
struct net_device *dev)
@@ -5461,9 +5463,9 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
entry = tnapi->tx_prod;
base_flags = 0;
- mss = 0;
- if ((mss = skb_shinfo(skb)->gso_size) != 0) {
- int tcp_opt_len, ip_tcp_len;
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
+ struct tcphdr *th;
u32 hdrlen;
if (skb_header_cloned(skb) &&
@@ -5471,18 +5473,16 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
dev_kfree_skb(skb);
goto out_unlock;
}
+ th = tcp_hdr(skb);
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
hdrlen = skb_headlen(skb) - ETH_HLEN;
else {
struct iphdr *iph = ip_hdr(skb);
- tcp_opt_len = tcp_optlen(skb);
- ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
+ hdrlen = ip_hdrlen(skb) + tcp_header_len_th(th);
+ iph->tot_len = htons(mss + hdrlen);
iph->check = 0;
- iph->tot_len = htons(mss + ip_tcp_len + tcp_opt_len);
- hdrlen = ip_tcp_len + tcp_opt_len;
}
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
@@ -5496,7 +5496,7 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb,
base_flags |= (TXD_FLAG_CPU_PRE_DMA |
TXD_FLAG_CPU_POST_DMA);
- tcp_hdr(skb)->check = 0;
+ th->check = 0;
}
else if (skb->ip_summed == CHECKSUM_PARTIAL)
@@ -5629,6 +5629,8 @@ tg3_tso_bug_end:
/* hard_start_xmit for devices that have the 4G bug and/or 40-bit bug and
* support TG3_FLG2_HW_TSO_1 or firmware TSO only.
+ *
+ * No TCP or IP length checking, per David Miller (see commit log).
*/
static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
struct net_device *dev)
@@ -5668,20 +5670,21 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
if (skb->ip_summed == CHECKSUM_PARTIAL)
base_flags |= TXD_FLAG_TCPUDP_CSUM;
- if ((mss = skb_shinfo(skb)->gso_size) != 0) {
+ mss = skb_shinfo(skb)->gso_size;
+ if (mss != 0) {
struct iphdr *iph;
- u32 tcp_opt_len, ip_tcp_len, hdr_len;
+ struct tcphdr *th;
+ u32 hdr_len;
+ int opt_bytes;
if (skb_header_cloned(skb) &&
pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {
dev_kfree_skb(skb);
goto out_unlock;
}
+ th = tcp_hdr(skb);
+ hdr_len = ip_hdrlen(skb) + tcp_header_len_th(th);
- tcp_opt_len = tcp_optlen(skb);
- ip_tcp_len = ip_hdrlen(skb) + sizeof(struct tcphdr);
-
- hdr_len = ip_tcp_len + tcp_opt_len;
if (unlikely((ETH_HLEN + hdr_len) > 80) &&
(tp->tg3_flags2 & TG3_FLG2_TSO_BUG))
return (tg3_tso_bug(tp, skb));
@@ -5693,13 +5696,14 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
iph->check = 0;
iph->tot_len = htons(mss + hdr_len);
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
- tcp_hdr(skb)->check = 0;
+ th->check = 0;
base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
} else
- tcp_hdr(skb)->check = ~csum_tcpudp_magic(iph->saddr,
- iph->daddr, 0,
- IPPROTO_TCP,
- 0);
+ th->check = ~csum_tcpudp_magic(iph->saddr, iph->daddr,
+ 0, IPPROTO_TCP, 0);
+
+ opt_bytes = hdr_len - sizeof(*iph) - sizeof(*th);
+ /* assumes positive opt_bytes without checking */
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO_3) {
mss |= (hdr_len & 0xc) << 12;
@@ -5710,19 +5714,11 @@ static netdev_tx_t tg3_start_xmit_dma_bug(struct sk_buff *skb,
mss |= hdr_len << 9;
else if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO_1) ||
GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705) {
- if (tcp_opt_len || iph->ihl > 5) {
- int tsflags;
-
- tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
- mss |= (tsflags << 11);
- }
+ if (opt_bytes > 0)
+ mss |= opt_bytes << (11 - 2);
} else {
- if (tcp_opt_len || iph->ihl > 5) {
- int tsflags;
-
- tsflags = (iph->ihl - 5) + (tcp_opt_len >> 2);
- base_flags |= tsflags << 12;
- }
+ if (opt_bytes > 0)
+ base_flags |= opt_bytes << (12 - 2);
}
}
#if TG3_VLAN_TAG_USED
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index d0133cf..74728f7 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -218,11 +218,6 @@ static inline unsigned int tcp_hdrlen(const struct sk_buff *skb)
return tcp_hdr(skb)->doff * 4;
}
-static inline unsigned int tcp_optlen(const struct sk_buff *skb)
-{
- return (tcp_hdr(skb)->doff - 5) * 4;
-}
-
/* Length of fixed header plus standard options. */
static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
{
--
1.6.3.3
^ permalink raw reply related
* [PATCH v3 1/7] net: tcp_header_len_th and tcp_option_len_th
From: William Allen Simpson @ 2010-02-14 6:05 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller
In-Reply-To: <4B7791CF.9030201@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 359 bytes --]
Redefine two TCP header functions to accept TCP header pointer.
When subtracting, return signed int to allow error checking.
These functions will be used in subsequent patches that implement
additional features.
Signed-off-by: William.Allen.Simpson@gmail.com
---
include/linux/tcp.h | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)
[-- Attachment #2: len_th+2a3+2.6.33-rc8.patch --]
[-- Type: text/plain, Size: 719 bytes --]
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 7fee8a4..d0133cf 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -223,6 +223,18 @@ static inline unsigned int tcp_optlen(const struct sk_buff *skb)
return (tcp_hdr(skb)->doff - 5) * 4;
}
+/* Length of fixed header plus standard options. */
+static inline unsigned int tcp_header_len_th(const struct tcphdr *th)
+{
+ return th->doff * 4;
+}
+
+/* Length of standard options only. This could be negative. */
+static inline int tcp_option_len_th(const struct tcphdr *th)
+{
+ return (int)(th->doff * 4) - sizeof(*th);
+}
+
/* This defines a selective acknowledgement block. */
struct tcp_sack_block_wire {
__be32 start_seq;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 0/7] tcp: bugs and cleanup updated to 2.6.33-rc8
From: William Allen Simpson @ 2010-02-14 6:01 UTC (permalink / raw)
To: Linux Kernel Developers, Linux Kernel Network Developers
Cc: Andrew Morton, David Miller
Combination of patches reported in October, November, December, January
and February. These patches fix perceived bugs, and other cleanup.
This code has had previous review and several months of limited testing.
Some portions were removed during the various TCPCT part 1 patch splits,
then were cut off by the sudden unexpected end of that merge window.
[03 Dec 2009] I've restarted the sub-numbering (again).
Of particular interest are the TCPCT header extensions that already
appear in the next phase of testing with other platforms. These patches
allow correct reception without data corruption.
The remainder of the original TCPCT part 2 will be merged with part 3.
These are patches against the current linux-2.6 tree.
[Parts 2f and 2g updated, improving resistance to very rare option
re-ordering by middleware.]
^ permalink raw reply
* Re: [PATCH] atm : fix /sys/devices/virtual/atm/X/carrier(ATM_PHY_SIG_UNKNOWN)
From: Chas Williams (CONTRACTOR) @ 2010-02-13 23:24 UTC (permalink / raw)
To: Philippe De Muyter; +Cc: netdev
In-Reply-To: <20100213215633.GA4345@frolo.macqel>
In message <20100213215633.GA4345@frolo.macqel>,Philippe De Muyter writes:
>value '1' while actually carrier was not yet established and real carrier
>state was not yet known to linux. I propose to fix that by using '?' as the
>/sys/devices/virtual/atm/cxacru0/carrier value when carrier state is not yet
>known to linux. Any other value except '1' would also be OK for me.
this is sort of intentional because some drivers dont actually implement
atm_dev->signal. ATM_PHY_SIG_UNKNOWN and ATM_PHY_SIG_LOST should
likely be carrier = 0. if the driver doesnt isnt going to handle
changing the state of atm_dev->signal it should just set the value to
ATM_PHY_SIG_FOUND.
i dont like the idea of carrier being '?' -- carrier is either true or false.
you have it or you dont.
^ permalink raw reply
* [PATCH] atm : fix /sys/devices/virtual/atm/X/carrier(ATM_PHY_SIG_UNKNOWN)
From: Philippe De Muyter @ 2010-02-13 21:56 UTC (permalink / raw)
To: chas, netdev
When trying to design udev rules to automate usage of cxacru usb atm (adsl)
modem, I have discovered that /sys/devices/virtual/atm/cxacru0/carrier had the
value '1' while actually carrier was not yet established and real carrier
state was not yet known to linux. I propose to fix that by using '?' as the
/sys/devices/virtual/atm/cxacru0/carrier value when carrier state is not yet
known to linux. Any other value except '1' would also be OK for me.
--
Currently, just after the interface creation,
/sys/devices/virtual/atm/cxacru0/carrier gives wrong info (carrier = 1),
while actually carrier is unknown. Fix that.
Signed-off-by: Philippe De Muyter <phdm@macqel.be>
--- a/net/atm/atm_sysfs.c 2010-02-13 22:54:04.598847195 +0100
+++ b/net/atm/atm_sysfs.c 2010-02-13 19:13:45.395308531 +0100
@@ -63,9 +63,9 @@ static ssize_t show_carrier(struct devic
char *pos = buf;
struct atm_dev *adev = to_atm_dev(cdev);
- pos += sprintf(pos, "%d\n",
- adev->signal ==
- adev->signal == ATM_PHY_SIG_LOST ? 0 : 1);
+ pos += sprintf(pos, "%c\n",
+ adev->signal == ATM_PHY_SIG_FOUND ? '1' :
+ adev->signal == ATM_PHY_SIG_LOST ? '0' : '?');
return pos - buf;
}
^ permalink raw reply
* Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support
From: Sridhar Samudrala @ 2010-02-13 20:55 UTC (permalink / raw)
To: Arnd Bergmann; +Cc: David Miller, Herbert Xu, netdev
In-Reply-To: <201002131834.00612.arnd@arndb.de>
On 2/13/2010 9:34 AM, Arnd Bergmann wrote:
> Hi Sridhar,
>
> On Saturday 13 February 2010, Sridhar Samudrala wrote:
>
>
>> This patch adds GSO/checksum offload support to macvtap driver and applies
>> on top of Arnd's refcnt bugfix.
>> http://patchwork.ozlabs.org/patch/45136/
>>
> Sorry for messing this up by replacing that patch with a different one.
> It shouldn't be hard to rebase this one though, which I'll probably do on Monday.
> Please tell me if you want to do it yourself instead.
>
No problem. If you get to it before, it is fine with me.
>> @@ -286,6 +288,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
>> sock_init_data(&q->sock,&q->sk);
>> q->sk.sk_allocation = GFP_ATOMIC; /* for now */
>> q->sk.sk_write_space = macvtap_sock_write_space;
>> + q->flags = IFF_VNET_HDR;
>>
>> err = macvtap_set_queue(dev, file, q);
>> if (err)
>>
> Making IFF_VNET_HDR the default probably prevents the driver from working
> with applications that don't known about VNET_HDR, e.g. anything other
> than qemu. I believe qemu always tries setting it though, which would make
> a default value of !IFF_NET_HDR fine.
>
Yes. It is better to make the default as !IFF_VNET_HDR
> Also, what about IFF_TAP and IFF_NO_PI, should those be always set?
>
Atleast it is not required for qemu to have these flags set. If we are
not doing anything different based on
these flags, i felt we don't need to have them.
>
>> @@ -499,18 +648,14 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
>> return 0;
>>
>> case TUNSETOFFLOAD:
>> - /* let the user check for future flags */
>> - if (arg& ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
>> - TUN_F_TSO_ECN | TUN_F_UFO))
>> - return -EINVAL;
>> -
>> - /* TODO: add support for these, so far we don't
>> - support any offload */
>> - if (arg& (TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
>> - TUN_F_TSO_ECN | TUN_F_UFO))
>> - return -EINVAL;
>> -
>> - return 0;
>> + q = macvtap_file_get_queue(file);
>> + if (!q)
>> + return -ENOLINK;
>> + ret = 0;
>> + if (!(q->flags& IFF_VNET_HDR))
>> + ret = -EINVAL;
>> + macvtap_file_put_queue(q);
>> + return ret;
>>
>> default:
>> return -EINVAL;
>>
> At least the first check needs to be in there, in case we are running with
> new user space that knows additional flags. Moreover, shouldn't we check
> the flags against the capabilities of vlan->lowerdev? I though it would be
> best to report the capabilities of the real hardware to the guest kernel
> so it can do the right thing.
>
Originally, i also thought we should check these based on the real
device capabilities. But later i realized,
that it is not really required as we fall back to software offload via
dev_gso_segment() call in dev_hard_start_xmit()
if the real device doesn't support any of the offloads. So we can
advertise that all the offloads are supported to
the guest and let host deal with any offloads that are not supported by
the real device.
Thanks
Sridhar
^ permalink raw reply
* [PATCH net-next-2.6 2/2] cxgb3: fix link flap
From: Divy Le Ray @ 2010-02-13 19:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, swise
In-Reply-To: <20100213194429.27297.99280.stgit@speedy5.asicdesigners.com>
From: Divy Le Ray <divy@chelsio.com>
The driver is expected to report that the link is up
when the phy Rx signal is established and the mac
has not detected a link fault.
The code is however broken, the driver does not check the link fault
status when the phy link status changes.
The link fault status being checked within a short period of time,
it leads to link up/link down events.
Signed-off-by: Divy Le Ray <divy@chelsio.com>
---
drivers/net/cxgb3/t3_hw.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/cxgb3/t3_hw.c b/drivers/net/cxgb3/t3_hw.c
index 032cfe0..3ab9f51 100644
--- a/drivers/net/cxgb3/t3_hw.c
+++ b/drivers/net/cxgb3/t3_hw.c
@@ -1262,7 +1262,8 @@ void t3_link_changed(struct adapter *adapter, int port_id)
lc->fc = fc;
}
- t3_os_link_changed(adapter, port_id, link_ok, speed, duplex, fc);
+ t3_os_link_changed(adapter, port_id, link_ok && !pi->link_fault,
+ speed, duplex, fc);
}
void t3_link_fault(struct adapter *adapter, int port_id)
^ permalink raw reply related
* [PATCH net-next-2.6 1/2] cxgb3: FIx VLAN over Jumbo frames
From: Divy Le Ray @ 2010-02-13 19:44 UTC (permalink / raw)
To: davem; +Cc: netdev, linux-kernel, swise
From: Divy Le Ray <divy@chelsio.com>
The mac is expected to auto-inflate the Maximum Frame size for VLAN
tagged frames. It however does not work with jumbo frames.
Work around the bug adding 4 to the Maximum Frame for MTUs
greater than 1536.
Signed-off-by: Divy Le Ray <divy@chelsio.com>
---
drivers/net/cxgb3/xgmac.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/cxgb3/xgmac.c b/drivers/net/cxgb3/xgmac.c
index 0109ee4..0c08de5 100644
--- a/drivers/net/cxgb3/xgmac.c
+++ b/drivers/net/cxgb3/xgmac.c
@@ -353,6 +353,9 @@ int t3_mac_set_mtu(struct cmac *mac, unsigned int mtu)
* packet size register includes header, but not FCS.
*/
mtu += 14;
+ if (mtu > 1536)
+ mtu += 4;
+
if (mtu > MAX_FRAME_SIZE - 4)
return -EINVAL;
t3_write_reg(adap, A_XGM_RX_MAX_PKT_SIZE + mac->offset, mtu);
^ permalink raw reply related
* [PATCH] p54usb: Add usbid for Corega CG-WLUSB2GT.
From: YOSHIFUJI Hideaki @ 2010-02-13 19:16 UTC (permalink / raw)
To: linux-wireless, netdev, linville, David Miller; +Cc: yoshfuji, hirofumi
From: Shimada Hirofumi <hirofumi@flycat.org>
p54usb: Add usbid for Corega CG-WLUSB2GT.
Signed-off-by: Shimada Hirofumi <hirofumi@flycat.org>
Signed-off-by: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
---
drivers/net/wireless/p54/p54usb.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/p54/p54usb.c b/drivers/net/wireless/p54/p54usb.c
index 92af9b9..dcb484b 100644
--- a/drivers/net/wireless/p54/p54usb.c
+++ b/drivers/net/wireless/p54/p54usb.c
@@ -36,6 +36,7 @@ static struct usb_device_id p54u_table[] __devinitdata = {
/* Version 1 devices (pci chip + net2280) */
{USB_DEVICE(0x0506, 0x0a11)}, /* 3COM 3CRWE254G72 */
{USB_DEVICE(0x0707, 0xee06)}, /* SMC 2862W-G */
+ {USB_DEVICE(0x07aa, 0x001c)}, /* Corega CG-WLUSB2GT */
{USB_DEVICE(0x083a, 0x4501)}, /* Accton 802.11g WN4501 USB */
{USB_DEVICE(0x083a, 0x4502)}, /* Siemens Gigaset USB Adapter */
{USB_DEVICE(0x083a, 0x5501)}, /* Phillips CPWUA054 */
^ permalink raw reply related
* [RFC]: IPv4 mroute interface referencing
From: David Lamparter @ 2010-02-13 18:29 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 1210 bytes --]
The attached patch introduces a new struct vifctln which is identical to
vifctl except for a new vifi_ifindex field. This allows creating mroute
vifs if you have multiple interfaces with the same IP (i.e.
"unnumbered", PtP interfaces) on your box.
Other ways to support this could be:
* new mroute API
+ it really needs an overhaul anyway
+ netlink would be more appropriate for mroute (IMHO)
- complicated, non-immediate fix
- complex user space software rewriting
* new MRT_ADD_VIF_EXT setsockopt
+ a bit more clean
- not needed really, sockopt mess
* stuff ifindex in lcl_addr field ("0.0.0.ifindex IP")
+ no new struct field
- idiotic (IMHO) API-monging
o compatibility gain if BSD did this, but BSD doesn't, no point
* add ifindex field (attached patch does this)
+ compatibility by setsockopt length parameter
+ doesn't break userspace software (none of the existing multicast
routers tries to pass excess junk)
- new struct, new struct field
This patch causes userspace-visible changes, but is fully backwards
compatible. Patch is against 2.6.32.8, I will provide an updated one if
discussion is in favour of something I can code up (i.e. not netlink).
Regards,
David
[-- Attachment #2: Type: text/x-patch, Size: 4419 bytes --]
>From 93e78e1d65976643ea8de54cfbe46f5741f49a9d Mon Sep 17 00:00:00 2001
From: David Lamparter <equinox@diac24.net>
Date: Sat, 13 Feb 2010 19:19:22 +0100
Subject: [PATCH] ipmr: new struct vifctln to support ifindex for MIF_ADD_VIF
it isn't currently possible to do multicast routing if multiple
interfaces with the same IP address are present in a system, as only the
first of them can be selected for mroute VIF creation.
fix this by adding struct vifctln (analogous to struct mreqn) which has
a new int vifi_ifindex field. the setsockopt call differentiates vifctl
from vifctln through the length field; non-present or zero vifi_ifindex
keeps the old behaviour, nonzero vifi_ifindex causes vifi_lcladdr to be
ignored in favour of vifi_ifindex.
Signed-off-by: David Lamparter <equinox@diac24.net>
---
include/linux/mroute.h | 14 ++++++++++++++
net/ipv4/ipmr.c | 19 +++++++++++++------
2 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/include/linux/mroute.h b/include/linux/mroute.h
index 08bc776..0ef73ce 100644
--- a/include/linux/mroute.h
+++ b/include/linux/mroute.h
@@ -63,6 +63,20 @@ struct vifctl {
struct in_addr vifc_rmt_addr; /* IPIP tunnel addr */
};
+/* Linux extension: reference VIF by ifindex.
+ * this struct MUST exactly retain the fields from vifctl for compatibility!
+ * vifc_ifindex is used if nonzero, otherwise fall back to old behaviour
+ */
+struct vifctln {
+ vifi_t vifc_vifi; /* Index of VIF */
+ unsigned char vifc_flags; /* VIFF_ flags */
+ unsigned char vifc_threshold; /* ttl limit */
+ unsigned int vifc_rate_limit; /* Rate limiter values (NI) */
+ struct in_addr vifc_lcl_addr; /* Our address */
+ struct in_addr vifc_rmt_addr; /* IPIP tunnel addr */
+ int vifc_ifindex; /* Index of real Interface */
+};
+
#define VIFF_TUNNEL 0x1 /* IPIP tunnel */
#define VIFF_SRCRT 0x2 /* NI */
#define VIFF_REGISTER 0x4 /* register vif */
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 99508d6..9d5d22e 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -23,6 +23,7 @@
* Carlos Picoto : PIMv1 Support
* Pavlin Ivanov Radoslavov: PIMv2 Registers must checksum only PIM header
* Relax this requrement to work with older peers.
+ * David Lamparter : struct vifctln introduced
*
*/
@@ -135,7 +136,7 @@ static void ipmr_del_tunnel(struct net_device *dev, struct vifctl *v)
}
static
-struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
+struct net_device *ipmr_new_tunnel(struct net *net, struct vifctln *v)
{
struct net_device *dev;
@@ -149,6 +150,7 @@ struct net_device *ipmr_new_tunnel(struct net *net, struct vifctl *v)
struct in_device *in_dev;
memset(&p, 0, sizeof(p));
+ p.link = v->vifc_ifindex;
p.iph.daddr = v->vifc_rmt_addr.s_addr;
p.iph.saddr = v->vifc_lcl_addr.s_addr;
p.iph.version = 4;
@@ -426,7 +428,7 @@ static void ipmr_update_thresholds(struct mfc_cache *cache, unsigned char *ttls)
}
}
-static int vif_add(struct net *net, struct vifctl *vifc, int mrtsock)
+static int vif_add(struct net *net, struct vifctln *vifc, int mrtsock)
{
int vifi = vifc->vifc_vifi;
struct vif_device *v = &net->ipv4.vif_table[vifi];
@@ -470,7 +472,10 @@ static int vif_add(struct net *net, struct vifctl *vifc, int mrtsock)
}
break;
case 0:
- dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
+ if (vifc->vifc_ifindex)
+ dev = dev_get_by_index(net, vifc->vifc_ifindex);
+ else
+ dev = ip_dev_find(net, vifc->vifc_lcl_addr.s_addr);
if (!dev)
return -EADDRNOTAVAIL;
err = dev_set_allmulti(dev, 1);
@@ -936,7 +941,7 @@ static void mrtsock_destruct(struct sock *sk)
int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsigned int optlen)
{
int ret;
- struct vifctl vif;
+ struct vifctln vif;
struct mfcctl mfc;
struct net *net = sock_net(sk);
@@ -975,9 +980,11 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, unsi
return ip_ra_control(sk, 0, NULL);
case MRT_ADD_VIF:
case MRT_DEL_VIF:
- if (optlen != sizeof(vif))
+ /* either vifctl or vifctln is fine; take what we get */
+ if (optlen != sizeof(vif) && optlen != sizeof(struct vifctl))
return -EINVAL;
- if (copy_from_user(&vif, optval, sizeof(vif)))
+ vif.vifc_ifindex = 0;
+ if (copy_from_user(&vif, optval, optlen))
return -EFAULT;
if (vif.vifc_vifi >= MAXVIFS)
return -ENFILE;
--
1.6.5.2
^ permalink raw reply related
* Re: [PATCH] vhost-net: switch to smp barriers
From: Michael S. Tsirkin @ 2010-02-13 17:39 UTC (permalink / raw)
To: Rusty Russell; +Cc: kvm, virtualization, netdev, linux-kernel, David Miller
In-Reply-To: <201002081849.39512.rusty@rustcorp.com.au>
On Mon, Feb 08, 2010 at 06:49:39PM +1030, Rusty Russell wrote:
> On Sun, 7 Feb 2010 07:37:49 pm Michael S. Tsirkin wrote:
> > On Mon, Feb 01, 2010 at 07:21:02PM +0200, Michael S. Tsirkin wrote:
> > > vhost-net only uses memory barriers to control SMP effects
> > > (communication with userspace potentially running on a different CPU),
> > > so it should use SMP barriers and not mandatory barriers for memory
> > > access ordering, as suggested by Documentation/memory-barriers.txt
> > >
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >
> >
> > Rusty, any feedback on this one?
> > Thanks!
>
> Yep. barrier() is correct on UP to guard against preemption.
>
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>
>
> Thanks,
> Rusty.
Dave, I see it's marked "not applicable":
http://patchwork.ozlabs.org/patch/44207/
the patch applies to net-next as of
b3b3f04fb587ecb61b5baa6c1c5f0e666fd12d73.
Can this be queued up please?
Should I resubmit with Rusty's ack?
Thanks!
--
MST
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 17:35 UTC (permalink / raw)
To: hadi; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1266078076.6776.40.camel@bigi>
Applied. Thanks.
jamal wrote:
> On Sun, 2010-02-14 at 00:47 +0900, YOSHIFUJI Hideaki wrote:
>> Hello.
>>
>> Okay, applied. Thank you.
>> Could you give me patch for doc/ping.sgml as well, please?
>
> Ok, here it is..
>
> cheers,
> jamal
>
^ permalink raw reply
* Re: [PATCH net-next-2.6] macvtap: Add GSO/csum offload support
From: Arnd Bergmann @ 2010-02-13 17:34 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: David Miller, Herbert Xu, netdev
In-Reply-To: <1266044316.10419.3.camel@w-sridhar.beaverton.ibm.com>
Hi Sridhar,
On Saturday 13 February 2010, Sridhar Samudrala wrote:
> This patch adds GSO/checksum offload support to macvtap driver and applies
> on top of Arnd's refcnt bugfix.
> http://patchwork.ozlabs.org/patch/45136/
Sorry for messing this up by replacing that patch with a different one.
It shouldn't be hard to rebase this one though, which I'll probably do on Monday.
Please tell me if you want to do it yourself instead.
> @@ -286,6 +288,7 @@ static int macvtap_open(struct inode *inode, struct file *file)
> sock_init_data(&q->sock, &q->sk);
> q->sk.sk_allocation = GFP_ATOMIC; /* for now */
> q->sk.sk_write_space = macvtap_sock_write_space;
> + q->flags = IFF_VNET_HDR;
>
> err = macvtap_set_queue(dev, file, q);
> if (err)
Making IFF_VNET_HDR the default probably prevents the driver from working
with applications that don't known about VNET_HDR, e.g. anything other
than qemu. I believe qemu always tries setting it though, which would make
a default value of !IFF_NET_HDR fine.
Also, what about IFF_TAP and IFF_NO_PI, should those be always set?
> @@ -499,18 +648,14 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
> return 0;
>
> case TUNSETOFFLOAD:
> - /* let the user check for future flags */
> - if (arg & ~(TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> - TUN_F_TSO_ECN | TUN_F_UFO))
> - return -EINVAL;
> -
> - /* TODO: add support for these, so far we don't
> - support any offload */
> - if (arg & (TUN_F_CSUM | TUN_F_TSO4 | TUN_F_TSO6 |
> - TUN_F_TSO_ECN | TUN_F_UFO))
> - return -EINVAL;
> -
> - return 0;
> + q = macvtap_file_get_queue(file);
> + if (!q)
> + return -ENOLINK;
> + ret = 0;
> + if (!(q->flags & IFF_VNET_HDR))
> + ret = -EINVAL;
> + macvtap_file_put_queue(q);
> + return ret;
>
> default:
> return -EINVAL;
At least the first check needs to be in there, in case we are running with
new user space that knows additional flags. Moreover, shouldn't we check
the flags against the capabilities of vlan->lowerdev? I though it would be
best to report the capabilities of the real hardware to the guest kernel
so it can do the right thing.
Arnd
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: jamal @ 2010-02-13 16:21 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev
In-Reply-To: <4B76C976.2060708@linux-ipv6.org>
[-- Attachment #1: Type: text/plain, Size: 194 bytes --]
On Sun, 2010-02-14 at 00:47 +0900, YOSHIFUJI Hideaki wrote:
> Hello.
>
> Okay, applied. Thank you.
> Could you give me patch for doc/ping.sgml as well, please?
Ok, here it is..
cheers,
jamal
[-- Attachment #2: ping.sgml-mark --]
[-- Type: text/plain, Size: 1342 bytes --]
commit 6e063d914a97c4b8160cb43a3129ea368b2a3fbb
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Sat Feb 13 11:19:13 2010 -0500
iputils: ping by mark doc update
Update documentation for ping by-mark
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/doc/ping.sgml b/doc/ping.sgml
index 5995ea9..c895d7d 100644
--- a/doc/ping.sgml
+++ b/doc/ping.sgml
@@ -16,6 +16,7 @@
<command>ping</command>
<arg choice="opt"><option>-LRUbdfnqrvVaAB</option></arg>
<arg choice="opt">-c <replaceable/count/</arg>
+<arg choice="opt">-m <replaceable/mark/</arg>
<arg choice="opt">-i <replaceable/interval/</arg>
<arg choice="opt">-l <replaceable/preload/</arg>
<arg choice="opt">-p <replaceable/pattern/</arg>
@@ -76,6 +77,14 @@ The address is bound to one selected when <command/ping/ starts.
</para></listitem>
</varlistentry>
<varlistentry>
+ <term><option>-m <replaceable/mark/</option></term>
+ <listitem><para>
+use <replaceable/mark/ to tag the packets going out. This is useful
+for variety of reasons within the kernel such as using policy
+routing to select specific outbound processing.
+ </para></listitem>
+ </varlistentry>
+ <varlistentry>
<term><option><anchor id="ping.count">-c <replaceable/count/</option></term>
<listitem><para>
Stop after sending <replaceable/count/ ECHO_REQUEST
^ permalink raw reply related
* Re: [net-next PATCH v3 3/3] net: TCP thin dupack
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: William Allen Simpson
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller, damian
In-Reply-To: <4B753943.2080800@gmail.com>
On 12. feb. 2010 12:19, William Allen Simpson wrote:
> Last year, I'm pretty sure I was on record as thinking this is *not* a
> good idea. But at least it now requires a sysctl to turn on, and
> should default to off.
>
> Also that naming was a bit dicey. Now the names are more descriptive,
> but the "force" is a bit overkill.
>
> How about:
> NET_TCP_FORCE_THIN_LINEAR_DUPACK -> NET_TCP_THIN_LINEAR_DUPACK
> tcp_force_thin_dupack -> tcp_thin_linear_dupack
> sysctl_tcp_force_thin_dupack -> sysctl_tcp_thin_linear_dupack
You uncovered a copy/paste/edit-typo there. The term "linear" had snuck
in even though it does not make sense for this patch. I think that
NET_TCP_THIN_DUPACK, tcp_thin_dupack and sysctl_tcp_thin_dupack will
be better.
Best regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-13 15:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Arnd Hannemann, LKML,
shemminger, David Miller, william.allen.simpson, damian
In-Reply-To: <1265946733.2891.10.camel@edumazet-laptop>
On 12. feb. 2010 04:52, Eric Dumazet wrote:
>
> Hi Anreas
>
> Could you include a section in Documentation/networking/ip-sysctl.txt
> about tcp_force_thin_linear_timeouts setting ?
>
I 'll submit a draft with the next iteration. thanks for the heads up on the procedure.
It may also be helpful with a separate textfile to describe the thin-stream latency
issues and the mechanisms. I will submit a draft for this also.
> Also, you should provide some documentation to be included in man pages
> (man 7 tcp), to Michael Kerrisk ( <mtk.manpages@gmail.com> ), both for
> tcp_force_thin_linear_timeouts and TCP_THIN_LT
>
> (Same applies for patch 3/3 and tcp_force_thin_dupack /
> TCP_THIN_DUPACK )
>
I'll make a draft, and mail him as soon as the naming issues are settled.
Best regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 2/3] net: TCP thin linear timeouts
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: William Allen Simpson
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller, damian
In-Reply-To: <4B753931.30405@gmail.com>
On 12. feb. 2010 12:19, William Allen Simpson wrote:
> Last year, I'm pretty sure I was on record as thinking this is only a
> marginally good idea, that would be better at the application layer.
>
> Also that naming was a bit dicey. Now the names are more descriptive,
> but the "force" is a bit overkill.
>
> How about?
> NET_TCP_FORCE_THIN_LINEAR_TIMEOUTS -> NET_TCP_THIN_LINEAR_TIMEOUTS
> TCP_THIN_LT -> TCP_THIN_LINEAR_TIMEOUTS
> TCP_THIN_LT_RETRIES -> TCP_THIN_LINEAR_RETRIES
> tcp_force_thin_linear_timeouts -> tcp_thin_linear_timeouts
> sysctl_tcp_force_thin_linear_timeouts -> sysctl_tcp_thin_linear_timeouts
> tp->thin_lt -> tp->thin_lto
>
> The latter mostly traditional "to" for "timeout", as used most everywhere.
>
I agree that the _force_-part should be taken out for both patches, and
renaming the lt to lto also makes sense. I'll fix it in the next iteration.
> Just for efficiency, I'd reorder this
> + if (sk->sk_state == TCP_ESTABLISHED &&
> + (tp->thin_lt || sysctl_tcp_force_thin_linear_timeouts) &&
> + tcp_stream_is_thin(sk) &&
> + icsk->icsk_retransmits <= TCP_THIN_LT_RETRIES) {
Thank you for this suggestion. I'll reorder in the next iteration.
Best regards,
Andreas
^ permalink raw reply
* Re: [net-next PATCH v3 3/3] net: TCP thin dupack
From: Andreas Petlund @ 2010-02-13 15:50 UTC (permalink / raw)
To: Eric W. Biederman
Cc: netdev@vger.kernel.org, Ilpo Järvinen, Eric Dumazet,
Arnd Hannemann, LKML, shemminger, David Miller,
william.allen.simpson, damian
In-Reply-To: <m16361ubly.fsf@fess.ebiederm.org>
On 13. feb. 2010 03:13, Eric W. Biederman wrote:
> Andreas Petlund <apetlund@simula.no> writes:
>
>> Major changes:
>> -Possible to disable mechanisms by socket option
>> -Socket option value boundary check
>>
>>
>> Signed-off-by: Andreas Petlund <apetlund@simula.no>
>> ---
>> include/linux/sysctl.h | 1 +
>> include/linux/tcp.h | 4 +++-
>> include/net/tcp.h | 1 +
>> net/ipv4/sysctl_net_ipv4.c | 7 +++++++
>> net/ipv4/tcp.c | 7 +++++++
>> net/ipv4/tcp_input.c | 11 +++++++++++
>> 6 files changed, 30 insertions(+), 1 deletions(-)
>>
>> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
>> index d840d75..ded3f20 100644
>> --- a/include/linux/sysctl.h
>> +++ b/include/linux/sysctl.h
>> @@ -426,6 +426,7 @@ enum
>> NET_TCP_MAX_SSTHRESH=124,
>> NET_TCP_FRTO_RESPONSE=125,
>> NET_TCP_FORCE_THIN_LINEAR_TIMEOUTS=126,
>> + NET_TCP_FORCE_THIN_LINEAR_DUPACK=127,
>
> There is no need to allocate a binary sysctl here.
>
> Eric
Thanks. I'll address this in the next iteration.
Best regrads,
Andreas
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 15:47 UTC (permalink / raw)
To: hadi; +Cc: netdev, YOSHIFUJI Hideaki
In-Reply-To: <1266074714.6776.28.camel@bigi>
Hello.
Okay, applied. Thank you.
Could you give me patch for doc/ping.sgml as well, please?
Regards,
--yoshfuji
jamal wrote:
> I am CCing this to netdev. Can someone help me poke on
> Yoshifuji-san? I cant reach him using electrons or
> complex physical molecules...
>
> cheers,
> jamal
>
>
>
^ permalink raw reply
* Re: [PATCH] iputils: ping by mark
From: YOSHIFUJI Hideaki @ 2010-02-13 15:38 UTC (permalink / raw)
To: hadi, YOSHIFUJI Hideaki, Linux Network Developers
In-Reply-To: <1266074714.6776.28.camel@bigi>
I notice this mail. I'll investigate the patch.
Thanks.
--yoshfuji
jamal wrote:
> I am CCing this to netdev. Can someone help me poke on
> Yoshifuji-san? I cant reach him using electrons or
> complex physical molecules...
>
> cheers,
> jamal
>
>
>
^ permalink raw reply
* [PATCH] iputils: ping by mark
From: jamal @ 2010-02-13 15:25 UTC (permalink / raw)
To: YOSHIFUJI Hideaki; +Cc: netdev
[-- Attachment #1: Type: text/plain, Size: 154 bytes --]
I am CCing this to netdev. Can someone help me poke on
Yoshifuji-san? I cant reach him using electrons or
complex physical molecules...
cheers,
jamal
[-- Attachment #2: ping-mark --]
[-- Type: text/plain, Size: 2747 bytes --]
commit 7afb1e52ecc8bda3677f8b7db8433486936d473f
Author: Jamal Hadi Salim <hadi@cyberus.ca>
Date: Mon Oct 12 16:59:27 2009 -0400
[PATCH] iputils: ping by mark
This extends ping to send a packet out based on a given
mark using -m option. Useful with policy routing to take different paths
to same destination ..
Signed-off-by: Jamal Hadi Salim <hadi@cyberus.ca>
diff --git a/ping.c b/ping.c
index b67cff4..5c913e0 100644
--- a/ping.c
+++ b/ping.c
@@ -1216,7 +1216,7 @@ void usage(void)
fprintf(stderr,
"Usage: ping [-LRUbdfnqrvVaA] [-c count] [-i interval] [-w deadline]\n"
" [-p pattern] [-s packetsize] [-t ttl] [-I interface or address]\n"
-" [-M mtu discovery hint] [-S sndbuf]\n"
+" [-M mtu discovery hint] [-m mark] [-S sndbuf]\n"
" [ -T timestamp option ] [ -Q tos ] [hop1 ...] destination\n");
exit(2);
}
diff --git a/ping_common.c b/ping_common.c
index be36cbd..b1cc9fc 100644
--- a/ping_common.c
+++ b/ping_common.c
@@ -4,6 +4,7 @@
int options;
+int mark;
int sndbuf;
int ttl;
int rtt;
@@ -141,6 +142,17 @@ void common_options(int ch)
options |= F_INTERVAL;
break;
}
+ case 'm':
+ {
+ char *endp;
+ mark = (int)strtoul(optarg, &endp, 10);
+ if (mark < 0 || *endp != '\0') {
+ fprintf(stderr, "mark cannot be negative");
+ exit(2);
+ }
+ options |= F_MARK;
+ break;
+ }
case 'w':
deadline = atoi(optarg);
if (deadline < 0) {
@@ -442,6 +454,15 @@ void setup(int icmp_sock)
fprintf(stderr, "Warning: no SO_TIMESTAMP support, falling back to SIOCGSTAMP\n");
}
#endif
+ if (options & F_MARK) {
+ if (setsockopt(icmp_sock, SOL_SOCKET, SO_MARK,
+ &mark, sizeof(mark)) == -1) {
+ /* we probably dont wanna exit since old kernels
+ * dont support mark ..
+ */
+ fprintf(stderr, "Warning: Failed to set mark %d\n", mark);
+ }
+ }
/* Set some SNDTIMEO to prevent blocking forever
* on sends, when device is too slow or stalls. Just put limit
diff --git a/ping_common.h b/ping_common.h
index 5b80118..466792e 100644
--- a/ping_common.h
+++ b/ping_common.h
@@ -60,6 +60,7 @@ extern int options;
#define F_STRICTSOURCE 0x8000
#define F_NOLOOP 0x10000
#define F_TTL 0x20000
+#define F_MARK 0x40000
/*
* MAX_DUP_CHK is the number of bits in received table, i.e. the maximum
@@ -118,9 +119,9 @@ case 'a': case 'U': case 'c': case 'd': \
case 'f': case 'i': case 'w': case 'l': \
case 'S': case 'n': case 'p': case 'q': \
case 'r': case 's': case 'v': case 'L': \
-case 't': case 'A': case 'W': case 'B':
+case 't': case 'A': case 'W': case 'B': case 'm':
-#define COMMON_OPTSTR "h?VQ:I:M:aUc:dfi:w:l:S:np:qrs:vLt:AW:B"
+#define COMMON_OPTSTR "h?VQ:I:M:aUc:dfi:w:l:S:np:qrs:vLt:AW:Bm:"
/*
^ permalink raw reply related
* [PATCH 2/2] net/macvtap: add vhost support
From: Arnd Bergmann @ 2010-02-13 10:35 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Ed Swierk, Patrick McHardy, Sridhar Samudrala,
qemu-devel, Michael S. Tsirkin
In-Reply-To: <201002131133.43028.arnd@arndb.de>
This adds support for passing a macvtap file descriptor into
vhost-net, much like we already do for tun/tap.
Most of the new code is taken from the respective patch
in the tun driver and may get consolidated in the future.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/macvtap.c | 98 ++++++++++++++++++++++++++++++++++---------
drivers/vhost/net.c | 8 +++-
include/linux/if_macvlan.h | 13 ++++++
3 files changed, 96 insertions(+), 23 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 7050997..e354501 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -58,6 +58,8 @@ static unsigned int macvtap_major;
static struct class *macvtap_class;
static struct cdev macvtap_cdev;
+static const struct proto_ops macvtap_socket_ops;
+
/*
* RCU usage:
* The macvtap_queue and the macvlan_dev are loosely coupled, the
@@ -176,7 +178,7 @@ static int macvtap_forward(struct net_device *dev, struct sk_buff *skb)
return -ENOLINK;
skb_queue_tail(&q->sk.sk_receive_queue, skb);
- wake_up(q->sk.sk_sleep);
+ wake_up_interruptible_poll(q->sk.sk_sleep, POLLIN | POLLRDNORM | POLLRDBAND);
return 0;
}
@@ -242,7 +244,7 @@ static void macvtap_sock_write_space(struct sock *sk)
return;
if (sk->sk_sleep && waitqueue_active(sk->sk_sleep))
- wake_up_interruptible_sync(sk->sk_sleep);
+ wake_up_interruptible_poll(sk->sk_sleep, POLLOUT | POLLWRNORM | POLLWRBAND);
}
static int macvtap_open(struct inode *inode, struct file *file)
@@ -270,6 +272,8 @@ static int macvtap_open(struct inode *inode, struct file *file)
init_waitqueue_head(&q->sock.wait);
q->sock.type = SOCK_RAW;
q->sock.state = SS_CONNECTED;
+ q->sock.file = file;
+ q->sock.ops = &macvtap_socket_ops;
sock_init_data(&q->sock, &q->sk);
q->sk.sk_write_space = macvtap_sock_write_space;
@@ -387,32 +391,20 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
rcu_read_lock_bh();
vlan = rcu_dereference(q->vlan);
- macvlan_count_rx(vlan, len, ret == 0, 0);
+ if (vlan)
+ macvlan_count_rx(vlan, len, ret == 0, 0);
rcu_read_unlock_bh();
return ret ? ret : len;
}
-static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
- unsigned long count, loff_t pos)
+static ssize_t macvtap_do_read(struct macvtap_queue *q, struct kiocb *iocb,
+ const struct iovec *iv, unsigned long len,
+ int noblock)
{
- struct file *file = iocb->ki_filp;
- struct macvtap_queue *q = file->private_data;
-
DECLARE_WAITQUEUE(wait, current);
struct sk_buff *skb;
- ssize_t len, ret = 0;
-
- if (!q) {
- ret = -ENOLINK;
- goto out;
- }
-
- len = iov_length(iv, count);
- if (len < 0) {
- ret = -EINVAL;
- goto out;
- }
+ ssize_t ret = 0;
add_wait_queue(q->sk.sk_sleep, &wait);
while (len) {
@@ -421,7 +413,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
/* Read frames from the queue */
skb = skb_dequeue(&q->sk.sk_receive_queue);
if (!skb) {
- if (file->f_flags & O_NONBLOCK) {
+ if (noblock) {
ret = -EAGAIN;
break;
}
@@ -440,7 +432,24 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
current->state = TASK_RUNNING;
remove_wait_queue(q->sk.sk_sleep, &wait);
+ return ret;
+}
+
+static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
+ unsigned long count, loff_t pos)
+{
+ struct file *file = iocb->ki_filp;
+ struct macvtap_queue *q = file->private_data;
+ ssize_t len, ret = 0;
+ len = iov_length(iv, count);
+ if (len < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ ret = macvtap_do_read(q, iocb, iv, len, file->f_flags & O_NONBLOCK);
+ ret = min_t(ssize_t, ret, len); /* XXX copied from tun.c. Why? */
out:
return ret;
}
@@ -538,6 +547,53 @@ static const struct file_operations macvtap_fops = {
#endif
};
+static int macvtap_sendmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *m, size_t total_len)
+{
+ struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
+ return macvtap_get_user(q, m->msg_iov, total_len,
+ m->msg_flags & MSG_DONTWAIT);
+}
+
+static int macvtap_recvmsg(struct kiocb *iocb, struct socket *sock,
+ struct msghdr *m, size_t total_len,
+ int flags)
+{
+ struct macvtap_queue *q = container_of(sock, struct macvtap_queue, sock);
+ int ret;
+ if (flags & ~(MSG_DONTWAIT|MSG_TRUNC))
+ return -EINVAL;
+ ret = macvtap_do_read(q, iocb, m->msg_iov, total_len,
+ flags & MSG_DONTWAIT);
+ if (ret > total_len) {
+ m->msg_flags |= MSG_TRUNC;
+ ret = flags & MSG_TRUNC ? ret : total_len;
+ }
+ return ret;
+}
+
+/* Ops structure to mimic raw sockets with tun */
+static const struct proto_ops macvtap_socket_ops = {
+ .sendmsg = macvtap_sendmsg,
+ .recvmsg = macvtap_recvmsg,
+};
+
+/* Get an underlying socket object from tun file. Returns error unless file is
+ * attached to a device. The returned object works like a packet socket, it
+ * can be used for sock_sendmsg/sock_recvmsg. The caller is responsible for
+ * holding a reference to the file for as long as the socket is in use. */
+struct socket *macvtap_get_socket(struct file *file)
+{
+ struct macvtap_queue *q;
+ if (file->f_op != &macvtap_fops)
+ return ERR_PTR(-EINVAL);
+ q = file->private_data;
+ if (!q)
+ return ERR_PTR(-EBADFD);
+ return &q->sock;
+}
+EXPORT_SYMBOL_GPL(macvtap_get_socket);
+
static int macvtap_init(void)
{
int err;
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 4c89283..91a324c 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -22,6 +22,7 @@
#include <linux/if_packet.h>
#include <linux/if_arp.h>
#include <linux/if_tun.h>
+#include <linux/if_macvlan.h>
#include <net/sock.h>
@@ -452,13 +453,16 @@ err:
return ERR_PTR(r);
}
-static struct socket *get_tun_socket(int fd)
+static struct socket *get_tap_socket(int fd)
{
struct file *file = fget(fd);
struct socket *sock;
if (!file)
return ERR_PTR(-EBADF);
sock = tun_get_socket(file);
+ if (!IS_ERR(sock))
+ return sock;
+ sock = macvtap_get_socket(file);
if (IS_ERR(sock))
fput(file);
return sock;
@@ -473,7 +477,7 @@ static struct socket *get_socket(int fd)
sock = get_raw_socket(fd);
if (!IS_ERR(sock))
return sock;
- sock = get_tun_socket(fd);
+ sock = get_tap_socket(fd);
if (!IS_ERR(sock))
return sock;
return ERR_PTR(-ENOTSOCK);
diff --git a/include/linux/if_macvlan.h b/include/linux/if_macvlan.h
index 51f1512..7d7f1e3 100644
--- a/include/linux/if_macvlan.h
+++ b/include/linux/if_macvlan.h
@@ -7,6 +7,19 @@
#include <linux/netlink.h>
#include <net/netlink.h>
+#if defined(CONFIG_MACVTAP) || defined(CONFIG_MACVTAP_MODULE)
+struct socket *macvtap_get_socket(struct file *);
+#else
+#include <linux/err.h>
+#include <linux/errno.h>
+struct file;
+struct socket;
+static inline struct socket *macvtap_get_socket(struct file *f)
+{
+ return ERR_PTR(-EINVAL);
+}
+#endif /* CONFIG_MACVTAP */
+
struct macvlan_port;
struct macvtap_queue;
--
1.6.3.3
^ permalink raw reply related
* [PATCH 1/2] macvtap: rework object lifetime rules
From: Arnd Bergmann @ 2010-02-13 10:33 UTC (permalink / raw)
To: Arnd Bergmann
Cc: netdev, linux-kernel, Ed Swierk, Patrick McHardy,
Sridhar Samudrala, qemu-devel, Michael S. Tsirkin
The original macvtap code has a number of problems
resulting from the use of RCU for protecting the
access to struct macvtap_queue from open files.
This includes
- need for GFP_ATOMIC allocations for skbs
- potential deadlocks when copy_*_user sleeps
- inability to work with vhost-net
Changing the lifetime of macvtap_queue to always
depend on the open file solves all these. The
RCU reference simply moves one step down to
the reference on the macvlan_dev, which we
only need for nonblocking operations.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/macvtap.c | 170 +++++++++++++++++++++++++-----------------------
1 files changed, 89 insertions(+), 81 deletions(-)
This patch replaces the previous one that cleans up the locking in a different way.
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index ad1f6ef..7050997 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -60,29 +60,19 @@ static struct cdev macvtap_cdev;
/*
* RCU usage:
- * The macvtap_queue is referenced both from the chardev struct file
- * and from the struct macvlan_dev using rcu_read_lock.
+ * The macvtap_queue and the macvlan_dev are loosely coupled, the
+ * pointers from one to the other can only be read while rcu_read_lock
+ * or macvtap_lock is held.
*
- * We never actually update the contents of a macvtap_queue atomically
- * with RCU but it is used for race-free destruction of a queue when
- * either the file or the macvlan_dev goes away. Pointers back to
- * the dev and the file are implicitly valid as long as the queue
- * exists.
+ * Both the file and the macvlan_dev hold a reference on the macvtap_queue
+ * through sock_hold(&q->sk). When the macvlan_dev goes away first,
+ * q->vlan becomes inaccessible. When the files gets closed,
+ * macvtap_get_queue() fails.
*
- * The callbacks from macvlan are always done with rcu_read_lock held
- * already, while in the file_operations, we get it ourselves.
- *
- * When destroying a queue, we remove the pointers from the file and
- * from the dev and then synchronize_rcu to make sure no thread is
- * still using the queue. There may still be references to the struct
- * sock inside of the queue from outbound SKBs, but these never
- * reference back to the file or the dev. The data structure is freed
- * through __sk_free when both our references and any pending SKBs
- * are gone.
- *
- * macvtap_lock is only used to prevent multiple concurrent open()
- * calls to assign a new vlan->tap pointer. It could be moved into
- * the macvlan_dev itself but is extremely rarely used.
+ * There may still be references to the struct sock inside of the
+ * queue from outbound SKBs, but these never reference back to the
+ * file or the dev. The data structure is freed through __sk_free
+ * when both our references and any pending SKBs are gone.
*/
static DEFINE_SPINLOCK(macvtap_lock);
@@ -100,11 +90,12 @@ static int macvtap_set_queue(struct net_device *dev, struct file *file,
goto out;
err = 0;
- q->vlan = vlan;
+ rcu_assign_pointer(q->vlan, vlan);
rcu_assign_pointer(vlan->tap, q);
+ sock_hold(&q->sk);
q->file = file;
- rcu_assign_pointer(file->private_data, q);
+ file->private_data = q;
out:
spin_unlock(&macvtap_lock);
@@ -112,28 +103,25 @@ out:
}
/*
- * We must destroy each queue exactly once, when either
- * the netdev or the file go away.
+ * The file owning the queue got closed, give up both
+ * the reference that the files holds as well as the
+ * one from the macvlan_dev if that still exists.
*
* Using the spinlock makes sure that we don't get
* to the queue again after destroying it.
- *
- * synchronize_rcu serializes with the packet flow
- * that uses rcu_read_lock.
*/
-static void macvtap_del_queue(struct macvtap_queue **qp)
+static void macvtap_put_queue(struct macvtap_queue *q)
{
- struct macvtap_queue *q;
+ struct macvlan_dev *vlan;
spin_lock(&macvtap_lock);
- q = rcu_dereference(*qp);
- if (!q) {
- spin_unlock(&macvtap_lock);
- return;
+ vlan = rcu_dereference(q->vlan);
+ if (vlan) {
+ rcu_assign_pointer(vlan->tap, NULL);
+ rcu_assign_pointer(q->vlan, NULL);
+ sock_put(&q->sk);
}
- rcu_assign_pointer(q->vlan->tap, NULL);
- rcu_assign_pointer(q->file->private_data, NULL);
spin_unlock(&macvtap_lock);
synchronize_rcu();
@@ -151,21 +139,29 @@ static struct macvtap_queue *macvtap_get_queue(struct net_device *dev,
return rcu_dereference(vlan->tap);
}
+/*
+ * The net_device is going away, give up the reference
+ * that it holds on the queue (all the queues one day)
+ * and safely set the pointer from the queues to NULL.
+ */
static void macvtap_del_queues(struct net_device *dev)
{
struct macvlan_dev *vlan = netdev_priv(dev);
- macvtap_del_queue(&vlan->tap);
-}
+ struct macvtap_queue *q;
-static inline struct macvtap_queue *macvtap_file_get_queue(struct file *file)
-{
- rcu_read_lock_bh();
- return rcu_dereference(file->private_data);
-}
+ spin_lock(&macvtap_lock);
+ q = rcu_dereference(vlan->tap);
+ if (!q) {
+ spin_unlock(&macvtap_lock);
+ return;
+ }
-static inline void macvtap_file_put_queue(void)
-{
- rcu_read_unlock_bh();
+ rcu_assign_pointer(vlan->tap, NULL);
+ rcu_assign_pointer(q->vlan, NULL);
+ spin_unlock(&macvtap_lock);
+
+ synchronize_rcu();
+ sock_put(&q->sk);
}
/*
@@ -275,7 +271,6 @@ static int macvtap_open(struct inode *inode, struct file *file)
q->sock.type = SOCK_RAW;
q->sock.state = SS_CONNECTED;
sock_init_data(&q->sock, &q->sk);
- q->sk.sk_allocation = GFP_ATOMIC; /* for now */
q->sk.sk_write_space = macvtap_sock_write_space;
err = macvtap_set_queue(dev, file, q);
@@ -291,13 +286,14 @@ out:
static int macvtap_release(struct inode *inode, struct file *file)
{
- macvtap_del_queue((struct macvtap_queue **)&file->private_data);
+ struct macvtap_queue *q = file->private_data;
+ macvtap_put_queue(q);
return 0;
}
static unsigned int macvtap_poll(struct file *file, poll_table * wait)
{
- struct macvtap_queue *q = macvtap_file_get_queue(file);
+ struct macvtap_queue *q = file->private_data;
unsigned int mask = POLLERR;
if (!q)
@@ -315,7 +311,6 @@ static unsigned int macvtap_poll(struct file *file, poll_table * wait)
mask |= POLLOUT | POLLWRNORM;
out:
- macvtap_file_put_queue();
return mask;
}
@@ -325,6 +320,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
int noblock)
{
struct sk_buff *skb;
+ struct macvlan_dev *vlan;
size_t len = count;
int err;
@@ -332,26 +328,37 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q,
return -EINVAL;
skb = sock_alloc_send_skb(&q->sk, NET_IP_ALIGN + len, noblock, &err);
-
- if (!skb) {
- macvlan_count_rx(q->vlan, 0, false, false);
- return err;
- }
+ if (!skb)
+ goto err;
skb_reserve(skb, NET_IP_ALIGN);
skb_put(skb, count);
- if (skb_copy_datagram_from_iovec(skb, 0, iv, 0, len)) {
- macvlan_count_rx(q->vlan, 0, false, false);
- kfree_skb(skb);
- return -EFAULT;
- }
+ err = skb_copy_datagram_from_iovec(skb, 0, iv, 0, len);
+ if (err)
+ goto err;
skb_set_network_header(skb, ETH_HLEN);
-
- macvlan_start_xmit(skb, q->vlan->dev);
+ rcu_read_lock_bh();
+ vlan = rcu_dereference(q->vlan);
+ if (vlan)
+ macvlan_start_xmit(skb, vlan->dev);
+ else
+ kfree_skb(skb);
+ rcu_read_unlock_bh();
return count;
+
+err:
+ rcu_read_lock_bh();
+ vlan = rcu_dereference(q->vlan);
+ if (vlan)
+ macvlan_count_rx(q->vlan, 0, false, false);
+ rcu_read_unlock_bh();
+
+ kfree_skb(skb);
+
+ return err;
}
static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
@@ -359,15 +366,10 @@ static ssize_t macvtap_aio_write(struct kiocb *iocb, const struct iovec *iv,
{
struct file *file = iocb->ki_filp;
ssize_t result = -ENOLINK;
- struct macvtap_queue *q = macvtap_file_get_queue(file);
-
- if (!q)
- goto out;
+ struct macvtap_queue *q = file->private_data;
result = macvtap_get_user(q, iv, iov_length(iv, count),
file->f_flags & O_NONBLOCK);
-out:
- macvtap_file_put_queue();
return result;
}
@@ -376,14 +378,17 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
const struct sk_buff *skb,
const struct iovec *iv, int len)
{
- struct macvlan_dev *vlan = q->vlan;
+ struct macvlan_dev *vlan;
int ret;
len = min_t(int, skb->len, len);
ret = skb_copy_datagram_const_iovec(skb, 0, iv, 0, len);
+ rcu_read_lock_bh();
+ vlan = rcu_dereference(q->vlan);
macvlan_count_rx(vlan, len, ret == 0, 0);
+ rcu_read_unlock_bh();
return ret ? ret : len;
}
@@ -392,7 +397,7 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
unsigned long count, loff_t pos)
{
struct file *file = iocb->ki_filp;
- struct macvtap_queue *q = macvtap_file_get_queue(file);
+ struct macvtap_queue *q = file->private_data;
DECLARE_WAITQUEUE(wait, current);
struct sk_buff *skb;
@@ -437,7 +442,6 @@ static ssize_t macvtap_aio_read(struct kiocb *iocb, const struct iovec *iv,
remove_wait_queue(q->sk.sk_sleep, &wait);
out:
- macvtap_file_put_queue();
return ret;
}
@@ -447,12 +451,13 @@ out:
static long macvtap_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
- struct macvtap_queue *q;
+ struct macvtap_queue *q = file->private_data;
+ struct macvlan_dev *vlan;
void __user *argp = (void __user *)arg;
struct ifreq __user *ifr = argp;
unsigned int __user *up = argp;
unsigned int u;
- char devname[IFNAMSIZ];
+ int ret;
switch (cmd) {
case TUNSETIFF:
@@ -464,16 +469,21 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
return 0;
case TUNGETIFF:
- q = macvtap_file_get_queue(file);
- if (!q)
+ rcu_read_lock_bh();
+ vlan = rcu_dereference(q->vlan);
+ if (vlan)
+ dev_hold(vlan->dev);
+ rcu_read_unlock_bh();
+
+ if (!vlan)
return -ENOLINK;
- memcpy(devname, q->vlan->dev->name, sizeof(devname));
- macvtap_file_put_queue();
+ ret = 0;
if (copy_to_user(&ifr->ifr_name, q->vlan->dev->name, IFNAMSIZ) ||
put_user((TUN_TAP_DEV | TUN_NO_PI), &ifr->ifr_flags))
- return -EFAULT;
- return 0;
+ ret = -EFAULT;
+ dev_put(vlan->dev);
+ return ret;
case TUNGETFEATURES:
if (put_user((IFF_TAP | IFF_NO_PI), up))
@@ -484,9 +494,7 @@ static long macvtap_ioctl(struct file *file, unsigned int cmd,
if (get_user(u, up))
return -EFAULT;
- q = macvtap_file_get_queue(file);
q->sk.sk_sndbuf = u;
- macvtap_file_put_queue();
return 0;
case TUNSETOFFLOAD:
--
1.6.3.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox