* Re: [PATCH v6 41/46] macvtap: TUN_VNET_HDR support
From: Jason Wang @ 2014-11-28 8:24 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Miller, cornelia.huck, rusty, nab, pbonzini,
thuth, dahi, Vlad Yasevich, Zhi Yong Wu, Tom Herbert,
Ben Hutchings, netdev
In-Reply-To: <1417118789-18231-42-git-send-email-mst@redhat.com>
On Fri, Nov 28, 2014 at 4:11 AM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/macvtap.c | 68
> ++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 43 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
> index 880cc09..af90ab5 100644
> --- a/drivers/net/macvtap.c
> +++ b/drivers/net/macvtap.c
> @@ -45,6 +45,18 @@ struct macvtap_queue {
> struct list_head next;
> };
>
> +#define MACVTAP_FEATURES (IFF_VNET_HDR | IFF_VNET_LE |
> IFF_MULTI_QUEUE)
> +
> +static inline u16 macvtap16_to_cpu(struct macvtap_queue *q,
> __virtio16 val)
> +{
> + return __virtio16_to_cpu(q->flags & IFF_VNET_LE, val);
> +}
> +
> +static inline __virtio16 cpu_to_macvtap16(struct macvtap_queue *q,
> u16 val)
> +{
> + return __cpu_to_virtio16(q->flags & IFF_VNET_LE, val);
> +}
> +
> static struct proto macvtap_proto = {
> .name = "macvtap",
> .owner = THIS_MODULE,
> @@ -557,7 +569,8 @@ static inline struct sk_buff
> *macvtap_alloc_skb(struct sock *sk, size_t prepad,
> * macvtap_skb_from_vnet_hdr and macvtap_skb_to_vnet_hdr should
> * be shared with the tun/tap driver.
> */
> -static int macvtap_skb_from_vnet_hdr(struct sk_buff *skb,
> +static int macvtap_skb_from_vnet_hdr(struct macvtap_queue *q,
> + struct sk_buff *skb,
> struct virtio_net_hdr *vnet_hdr)
> {
> unsigned short gso_type = 0;
> @@ -588,13 +601,13 @@ static int macvtap_skb_from_vnet_hdr(struct
> sk_buff *skb,
> }
>
> if (vnet_hdr->flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> - if (!skb_partial_csum_set(skb, vnet_hdr->csum_start,
> - vnet_hdr->csum_offset))
> + if (!skb_partial_csum_set(skb, macvtap16_to_cpu(q,
> vnet_hdr->csum_start),
> + macvtap16_to_cpu(q, vnet_hdr->csum_offset)))
> return -EINVAL;
> }
>
> if (vnet_hdr->gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> - skb_shinfo(skb)->gso_size = vnet_hdr->gso_size;
> + skb_shinfo(skb)->gso_size = macvtap16_to_cpu(q,
> vnet_hdr->gso_size);
> skb_shinfo(skb)->gso_type = gso_type;
>
> /* Header must be checked, and gso_segs computed. */
> @@ -604,8 +617,9 @@ static int macvtap_skb_from_vnet_hdr(struct
> sk_buff *skb,
> return 0;
> }
>
> -static void macvtap_skb_to_vnet_hdr(const struct sk_buff *skb,
> - struct virtio_net_hdr *vnet_hdr)
> +static void macvtap_skb_to_vnet_hdr(struct macvtap_queue *q,
> + const struct sk_buff *skb,
> + struct virtio_net_hdr *vnet_hdr)
> {
> memset(vnet_hdr, 0, sizeof(*vnet_hdr));
>
> @@ -613,8 +627,8 @@ static void macvtap_skb_to_vnet_hdr(const struct
> sk_buff *skb,
> struct skb_shared_info *sinfo = skb_shinfo(skb);
>
> /* This is a hint as to how much should be linear. */
> - vnet_hdr->hdr_len = skb_headlen(skb);
> - vnet_hdr->gso_size = sinfo->gso_size;
> + vnet_hdr->hdr_len = cpu_to_macvtap16(q, skb_headlen(skb));
> + vnet_hdr->gso_size = cpu_to_macvtap16(q, sinfo->gso_size);
> if (sinfo->gso_type & SKB_GSO_TCPV4)
> vnet_hdr->gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> else if (sinfo->gso_type & SKB_GSO_TCPV6)
> @@ -628,10 +642,13 @@ static void macvtap_skb_to_vnet_hdr(const
> struct sk_buff *skb,
>
> if (skb->ip_summed == CHECKSUM_PARTIAL) {
> vnet_hdr->flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
> - vnet_hdr->csum_start = skb_checksum_start_offset(skb);
> if (vlan_tx_tag_present(skb))
> - vnet_hdr->csum_start += VLAN_HLEN;
> - vnet_hdr->csum_offset = skb->csum_offset;
> + vnet_hdr->csum_start = cpu_to_macvtap16(q,
> + skb_checksum_start_offset(skb) + VLAN_HLEN);
> + else
> + vnet_hdr->csum_start = cpu_to_macvtap16(q,
> + skb_checksum_start_offset(skb));
> + vnet_hdr->csum_offset = cpu_to_macvtap16(q, skb->csum_offset);
> } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
> vnet_hdr->flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
> @@ -666,12 +683,14 @@ static ssize_t macvtap_get_user(struct
> macvtap_queue *q, struct msghdr *m,
> if (err < 0)
> goto err;
> if ((vnet_hdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
> - vnet_hdr.csum_start + vnet_hdr.csum_offset + 2 >
> - vnet_hdr.hdr_len)
> - vnet_hdr.hdr_len = vnet_hdr.csum_start +
> - vnet_hdr.csum_offset + 2;
> + macvtap16_to_cpu(q, vnet_hdr.csum_start) +
> + macvtap16_to_cpu(q, vnet_hdr.csum_offset) + 2 >
> + macvtap16_to_cpu(q, vnet_hdr.hdr_len))
> + vnet_hdr.hdr_len = cpu_to_macvtap16(q,
> + macvtap16_to_cpu(q, vnet_hdr.csum_start) +
> + macvtap16_to_cpu(q, vnet_hdr.csum_offset) + 2);
> err = -EINVAL;
> - if (vnet_hdr.hdr_len > len)
> + if (macvtap16_to_cpu(q, vnet_hdr.hdr_len) > len)
> goto err;
> }
>
> @@ -684,7 +703,8 @@ static ssize_t macvtap_get_user(struct
> macvtap_queue *q, struct msghdr *m,
> goto err;
>
> if (m && m->msg_control && sock_flag(&q->sk, SOCK_ZEROCOPY)) {
> - copylen = vnet_hdr.hdr_len ? vnet_hdr.hdr_len : GOODCOPY_LEN;
> + copylen = vnet_hdr.hdr_len ?
> + macvtap16_to_cpu(q, vnet_hdr.hdr_len) : GOODCOPY_LEN;
> if (copylen > good_linear)
> copylen = good_linear;
> linear = copylen;
> @@ -695,10 +715,10 @@ static ssize_t macvtap_get_user(struct
> macvtap_queue *q, struct msghdr *m,
>
> if (!zerocopy) {
> copylen = len;
> - if (vnet_hdr.hdr_len > good_linear)
> + if (macvtap16_to_cpu(q, vnet_hdr.hdr_len) > good_linear)
> linear = good_linear;
> else
> - linear = vnet_hdr.hdr_len;
> + linear = macvtap16_to_cpu(q, vnet_hdr.hdr_len);
> }
>
> skb = macvtap_alloc_skb(&q->sk, NET_IP_ALIGN, copylen,
> @@ -725,7 +745,7 @@ static ssize_t macvtap_get_user(struct
> macvtap_queue *q, struct msghdr *m,
> skb->protocol = eth_hdr(skb)->h_proto;
>
> if (vnet_hdr_len) {
> - err = macvtap_skb_from_vnet_hdr(skb, &vnet_hdr);
> + err = macvtap_skb_from_vnet_hdr(q, skb, &vnet_hdr);
> if (err)
> goto err_kfree;
> }
> @@ -791,7 +811,7 @@ static ssize_t macvtap_put_user(struct
> macvtap_queue *q,
> if ((len -= vnet_hdr_len) < 0)
> return -EINVAL;
>
> - macvtap_skb_to_vnet_hdr(skb, &vnet_hdr);
> + macvtap_skb_to_vnet_hdr(q, skb, &vnet_hdr);
>
> if (memcpy_toiovecend(iv, (void *)&vnet_hdr, 0, sizeof(vnet_hdr)))
> return -EFAULT;
> @@ -1003,8 +1023,7 @@ static long macvtap_ioctl(struct file *file,
> unsigned int cmd,
> return -EFAULT;
>
> ret = 0;
> - if ((u & ~(IFF_VNET_HDR | IFF_MULTI_QUEUE)) !=
> - (IFF_NO_PI | IFF_TAP))
> + if ((u & ~MACVTAP_FEATURES) != (IFF_NO_PI | IFF_TAP))
> ret = -EINVAL;
> else
> q->flags = u;
> @@ -1036,8 +1055,7 @@ static long macvtap_ioctl(struct file *file,
> unsigned int cmd,
> return ret;
>
> case TUNGETFEATURES:
> - if (put_user(IFF_TAP | IFF_NO_PI | IFF_VNET_HDR |
> - IFF_MULTI_QUEUE, up))
> + if (put_user(IFF_TAP | IFF_NO_PI | MACVTAP_FEATURES, up))
> return -EFAULT;
> return 0;
>
> --
> MST
Reviewed-by: Jason Wang <jasowang@redhat.com>
btw, title should be macvtap: IFF_VNET_LE support
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-11-28 8:24 UTC (permalink / raw)
To: Kevin Zhu
Cc: Alex Strizhevsky, Bjørn Mork, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Eli Britstein
In-Reply-To: <54781523.6030600@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 7444 bytes --]
The driver effectively uses the wNdpOutDivisor indirectly - see standard
cdc_ncm deriver as in kernel git tree, line 490.
On Fri, 28 Nov 2014, Kevin Zhu wrote:
==Date: Fri, 28 Nov 2014 07:24:49
==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
==To: Alex Strizhevsky <alexxst@gmail.com>, Bjørn Mork <bjorn@mork.no>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>
==Cc: Enrico Mioso <mrkiko.rs@gmail.com>, "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
== Eli Britstein <Eli.Britstein@audiocodes.com>
==Subject: Re: Is this 32-bit NCM?
==
==Hi all,
==
==I'm able to get the following prints with the original huawei_cdc_ncm driver
==from Ubuntu 12.04.3. It keeps on printing. I'm also able to get an IP, but
==no Internet access.
==
==^HCSQ:"WCDMA",64,59,55
==
==^DSFLOWRPT:0000000E,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==OK
==
==^NDISSTAT:1,,,"IPV4"
==
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:00000010,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000012,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000014,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^STIN: 1, 0, 0
==
==
==...
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:0000008C,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:0000008E,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
==Regarding the alignment, I think we have some difference between Windows and
==Linux. The spec says it should satisfy the following constraint.
==
==Offset % wNdpOutDivisor == wNdpOutPayloadRemainder (for OUT datagrams)
==
==It seems the Huawei device take the NDP offset (Ethernet Header Offset) as
==the parameter 'Offset' in the above constraint. And in the Linux capture, it
==does not comply with it.
==
==
==dwNtbInMaxSize=131072 dwNtbOutMaxSize=16384 wNdpOutPayloadRemainder=2
==wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=0 flags=0x1f
==
==Windows:
==
==0000 6e 63 6d 68 10 00 b6 00 7c 00 00 00 5c 00 00 00 ncmh....|...\...
==0010 00 00 00 00 00 00 00 00 00 1e 10 1f 00 00 08 00 ................
==0020 45 00 00 3c 00 9b 00 00 80 01 a0 fe 0a 71 a3 0e E..<.........q..
==0030 ca 6c 21 3c 08 00 4b 4b 00 01 02 10 61 62 63 64 .l!<..KK....abcd
==0040 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 efghijklmnopqrst
==0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
==0060 20 00 00 00 00 00 00 00 00 00 00 00 12 00 00 00 ...............
==0070 4a 00 00 00 00 00 00 00 00 00 00 00 J...........
==
==Linux: (thought it's a 16bit format capture, but it's the same as 32bit,
==regarding the alignment)
==
==0000 4e 43 4d 48 0c 00 40 00 e2 00 0c 00 4e 43 4d 30 NCMH..@.....NCM0
==0010 10 00 00 00 b8 00 2a 00 00 00 00 00 00 00 00 00 ......*.........
==0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0090 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00b0 00 00 00 00 00 00 00 00 ff ff ff ff ff ff 00 1e ................
==00c0 10 1f 00 00 08 06 00 01 08 00 06 04 00 01 00 1e ................
==00d0 10 1f 00 00 0a 71 cc a6 00 00 00 00 00 00 70 50 .....q........pP
==00e0 f8 49 .I
==
==
==Regards,
==Kevin
==On 11/27/2014 08:36 PM, Alex Strizhevsky wrote:
== Adding my colleagues - Eli, Kevin & Midge.
==
==Any ideas are welcome ;)
==
==
==On Thu, Nov 27, 2014 at 12:03 PM, Bjørn Mork <bjorn@mork.no> wrote:
== Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
== > Ok - we can arrive to some ocnclusions regarding the
== E3272.
== > First of all - the modem seems buggy enough to not be
== able to handle requests
== > for different formats. You need to unplug and re-plug
== it, but this is onlyan
== > impression and is reasonable.
== >
== > Then - the modem will accept to ndisdup the connection
== with
== > at^ndisdup=1,1,"internet"
== > but - if we use huawei_cdc_ncm + cdc_ncm we have no flow
== handling messages and
== > the modem stops here.
== > If we use the cdc_ncm 32-bit driver (modified) we get
== lotfs of
== > ^dsflorpt
== > that's how it should be.
== > So I think we can say that something is changing.
== > Then there's the alignment problem you mentioned in your
== previous reply. And
== > this is hard to solve.
== > could you try to help me understand where the problem
== is?
== > I feel like we are very close to the solution but
== something isn't working.
== > Or might be just try to change the 16 bit driver?
==
==If you use a recent version of the driver as a basis, then you
==get the
==CDC NCM NTB parameters in sysfs (if not, then you need to enable
==debugging and look in the log for these values). For example:
==
==bjorn@nemi:~$ grep . /sys/class/net/wwan0/cdc_ncm/*
==/sys/class/net/wwan0/cdc_ncm/bmNtbFormatsSupported:0x0001
==/sys/class/net/wwan0/cdc_ncm/dwNtbInMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/dwNtbOutMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/min_tx_pkt:13824
==/sys/class/net/wwan0/cdc_ncm/rx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_timer_usecs:400
==/sys/class/net/wwan0/cdc_ncm/wNdpInAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpInDivisor:1
==/sys/class/net/wwan0/cdc_ncm/wNdpInPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNdpOutAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpOutDivisor:32
==/sys/class/net/wwan0/cdc_ncm/wNdpOutPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNtbOutMaxDatagrams:32
==
==
==The possible problem I am thinking of is proper handling of the
==wNdp*PayloadRemainder values. See section 3.3.4 "NCM Ethernet
==Frame
==Alignment" in the spec. Which is confusing as hell, but if I
==understand
==it correctly then we are supposed to align the start of the IP
==packets
==(the "payload", _not_ the ethernet frame) to a whole
==wNdp*Divisor number
==as long as the wNdp*PayloadRemainder is 0.
==
==
==Bjørn
==
==
==
==This email and any files transmitted with it are confidential material. They
==are intended solely for the use of the designated individual or entity to
==whom they are addressed. If the reader of this message is not the intended
==recipient, you are hereby notified that any dissemination, use, distribution
==or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the
==sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Kevin Zhu @ 2014-11-28 8:23 UTC (permalink / raw)
To: Enrico Mioso
Cc: Alex Strizhevsky, Bjørn Mork, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Eli Britstein
In-Reply-To: <alpine.LNX.2.03.1411280915310.3981@gmail.com>
It's an ping packet. Windows pads it with 'abcdef...'.
Regards,
Kevin
On 11/28/2014 04:17 PM, Enrico Mioso wrote:
> And...why in Windows we have this line?
> 0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
> Is this just padding stuff before the "ncm0" string?
> So in particular a simple line needs to be modified in practice in the driver,
> or am I totally misguised?
> Is our "offset" parameter calculatedsuitably in the driver? Looking at it, any
> comment appreciated.
>
>
> On Fri, 28 Nov 2014, Kevin Zhu wrote:
>
> ==Date: Fri, 28 Nov 2014 07:24:49
> ==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
> ==To: Alex Strizhevsky <alexxst@gmail.com>, Bjørn Mork <bjorn@mork.no>,
> == Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>
> ==Cc: Enrico Mioso <mrkiko.rs@gmail.com>, "youtux@gmail.com" <youtux@gmail.com>,
> == "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
> == "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
> == Eli Britstein <Eli.Britstein@audiocodes.com>
> ==Subject: Re: Is this 32-bit NCM?
> ==
> ==Hi all,
> ==
> ==I'm able to get the following prints with the original huawei_cdc_ncm driver
> ==from Ubuntu 12.04.3. It keeps on printing. I'm also able to get an IP, but
> ==no Internet access.
> ==
> ==^HCSQ:"WCDMA",64,59,55
> ==
> ==^DSFLOWRPT:0000000E,00000000,00000000,0000000000000000,0000000000000000,000
> ==00000,00000000
> ==
> ==OK
> ==
> ==^NDISSTAT:1,,,"IPV4"
> ==
> ==^STIN: 1, 0, 0
> ==
> ==^DSFLOWRPT:00000010,00000000,00000000,0000000000000000,0000000000000000,000
> ==00000,00000000
> ==
> ==^DSFLOWRPT:00000012,00000000,00000000,0000000000000000,0000000000000000,000
> ==00000,00000000
> ==
> ==^DSFLOWRPT:00000014,00000000,00000000,0000000000000000,0000000000000000,000
> ==00000,00000000
> ==
> ==^STIN: 1, 0, 0
> ==
> ==
> ==...
> ==^STIN: 1, 0, 0
> ==
> ==^DSFLOWRPT:0000008C,00000000,00000000,0000000000000B36,0000000000000000,000
> ==00000,00000000
> ==
> ==^DSFLOWRPT:0000008E,00000000,00000000,0000000000000B36,0000000000000000,000
> ==00000,00000000
> ==
> ==Regarding the alignment, I think we have some difference between Windows and
> ==Linux. The spec says it should satisfy the following constraint.
> ==
> ==Offset % wNdpOutDivisor == wNdpOutPayloadRemainder (for OUT datagrams)
> ==
> ==It seems the Huawei device take the NDP offset (Ethernet Header Offset) as
> ==the parameter 'Offset' in the above constraint. And in the Linux capture, it
> ==does not comply with it.
> ==
> ==
> ==dwNtbInMaxSize=131072 dwNtbOutMaxSize=16384 wNdpOutPayloadRemainder=2
> ==wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=0 flags=0x1f
> ==
> ==Windows:
> ==
> ==0000 6e 63 6d 68 10 00 b6 00 7c 00 00 00 5c 00 00 00 ncmh....|...\...
> ==0010 00 00 00 00 00 00 00 00 00 1e 10 1f 00 00 08 00 ................
> ==0020 45 00 00 3c 00 9b 00 00 80 01 a0 fe 0a 71 a3 0e E..<.........q..
> ==0030 ca 6c 21 3c 08 00 4b 4b 00 01 02 10 61 62 63 64 .l!<..KK....abcd
> ==0040 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 efghijklmnopqrst
> ==0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
> ==0060 20 00 00 00 00 00 00 00 00 00 00 00 12 00 00 00 ...............
> ==0070 4a 00 00 00 00 00 00 00 00 00 00 00 J...........
> ==
> ==Linux: (thought it's a 16bit format capture, but it's the same as 32bit,
> ==regarding the alignment)
> ==
> ==0000 4e 43 4d 48 0c 00 40 00 e2 00 0c 00 4e 43 4d 30 NCMH..@.....NCM0
> ==0010 10 00 00 00 b8 00 2a 00 00 00 00 00 00 00 00 00 ......*.........
> ==0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==0090 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==00a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> ==00b0 00 00 00 00 00 00 00 00 ff ff ff ff ff ff 00 1e ................
> ==00c0 10 1f 00 00 08 06 00 01 08 00 06 04 00 01 00 1e ................
> ==00d0 10 1f 00 00 0a 71 cc a6 00 00 00 00 00 00 70 50 .....q........pP
> ==00e0 f8 49 .I
> ==
> ==
> ==Regards,
> ==Kevin
> ==On 11/27/2014 08:36 PM, Alex Strizhevsky wrote:
> == Adding my colleagues - Eli, Kevin & Midge.
> ==
> ==Any ideas are welcome ;)
> ==
> ==
> ==On Thu, Nov 27, 2014 at 12:03 PM, Bjørn Mork <bjorn@mork.no> wrote:
> == Enrico Mioso <mrkiko.rs@gmail.com> writes:
> ==
> == > Ok - we can arrive to some ocnclusions regarding the
> == E3272.
> == > First of all - the modem seems buggy enough to not be
> == able to handle requests
> == > for different formats. You need to unplug and re-plug
> == it, but this is onlyan
> == > impression and is reasonable.
> == >
> == > Then - the modem will accept to ndisdup the connection
> == with
> == > at^ndisdup=1,1,"internet"
> == > but - if we use huawei_cdc_ncm + cdc_ncm we have no flow
> == handling messages and
> == > the modem stops here.
> == > If we use the cdc_ncm 32-bit driver (modified) we get
> == lotfs of
> == > ^dsflorpt
> == > that's how it should be.
> == > So I think we can say that something is changing.
> == > Then there's the alignment problem you mentioned in your
> == previous reply. And
> == > this is hard to solve.
> == > could you try to help me understand where the problem
> == is?
> == > I feel like we are very close to the solution but
> == something isn't working.
> == > Or might be just try to change the 16 bit driver?
> ==
> ==If you use a recent version of the driver as a basis, then you
> ==get the
> ==CDC NCM NTB parameters in sysfs (if not, then you need to enable
> ==debugging and look in the log for these values). For example:
> ==
> ==bjorn@nemi:~$ grep . /sys/class/net/wwan0/cdc_ncm/*
> ==/sys/class/net/wwan0/cdc_ncm/bmNtbFormatsSupported:0x0001
> ==/sys/class/net/wwan0/cdc_ncm/dwNtbInMaxSize:15360
> ==/sys/class/net/wwan0/cdc_ncm/dwNtbOutMaxSize:15360
> ==/sys/class/net/wwan0/cdc_ncm/min_tx_pkt:13824
> ==/sys/class/net/wwan0/cdc_ncm/rx_max:15360
> ==/sys/class/net/wwan0/cdc_ncm/tx_max:15360
> ==/sys/class/net/wwan0/cdc_ncm/tx_timer_usecs:400
> ==/sys/class/net/wwan0/cdc_ncm/wNdpInAlignment:4
> ==/sys/class/net/wwan0/cdc_ncm/wNdpInDivisor:1
> ==/sys/class/net/wwan0/cdc_ncm/wNdpInPayloadRemainder:0
> ==/sys/class/net/wwan0/cdc_ncm/wNdpOutAlignment:4
> ==/sys/class/net/wwan0/cdc_ncm/wNdpOutDivisor:32
> ==/sys/class/net/wwan0/cdc_ncm/wNdpOutPayloadRemainder:0
> ==/sys/class/net/wwan0/cdc_ncm/wNtbOutMaxDatagrams:32
> ==
> ==
> ==The possible problem I am thinking of is proper handling of the
> ==wNdp*PayloadRemainder values. See section 3.3.4 "NCM Ethernet
> ==Frame
> ==Alignment" in the spec. Which is confusing as hell, but if I
> ==understand
> ==it correctly then we are supposed to align the start of the IP
> ==packets
> ==(the "payload", _not_ the ethernet frame) to a whole
> ==wNdp*Divisor number
> ==as long as the wNdp*PayloadRemainder is 0.
> ==
> ==
> ==Bjørn
> ==
> ==
> ==
> ==This email and any files transmitted with it are confidential material. They
> ==are intended solely for the use of the designated individual or entity to
> ==whom they are addressed. If the reader of this message is not the intended
> ==recipient, you are hereby notified that any dissemination, use, distribution
> ==or copying of this communication is strictly prohibited and may be unlawful.
> ==
> ==If you have received this email in error please immediately notify the
> ==sender and delete or destroy any copy of this message
> ==
This email and any files transmitted with it are confidential material. They are intended solely for the use of the designated individual or entity to whom they are addressed. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, use, distribution or copying of this communication is strictly prohibited and may be unlawful.
If you have received this email in error please immediately notify the sender and delete or destroy any copy of this message
^ permalink raw reply
* Re: [PATCH v6 40/46] tun: TUN_VNET_LE support, fix sparse warnings for virtio headers
From: Jason Wang @ 2014-11-28 8:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Miller, cornelia.huck, rusty, nab, pbonzini,
thuth, dahi, Zhi Yong Wu, Ben Hutchings, Tom Herbert,
Masatake YAMATO, Herbert Xu, Xi Wang, netdev
In-Reply-To: <1417118789-18231-41-git-send-email-mst@redhat.com>
On Fri, Nov 28, 2014 at 4:11 AM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> Pretty straight-forward: convert all fields to/from
> virtio endian-ness.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/tun.c | 48
> +++++++++++++++++++++++++++++-------------------
> 1 file changed, 29 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 61c000c..f411ffd 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -111,7 +111,7 @@ do { \
> #define TUN_FASYNC IFF_ATTACH_QUEUE
>
> #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> - IFF_MULTI_QUEUE)
> + IFF_VNET_LE | IFF_MULTI_QUEUE)
> #define GOODCOPY_LEN 128
>
> #define FLT_EXACT_COUNT 8
> @@ -205,6 +205,16 @@ struct tun_struct {
> u32 flow_count;
> };
>
> +static inline u16 tun16_to_cpu(struct tun_struct *tun, __virtio16
> val)
> +{
> + return __virtio16_to_cpu(tun->flags & IFF_VNET_LE, val);
> +}
> +
> +static inline __virtio16 cpu_to_tun16(struct tun_struct *tun, u16
> val)
> +{
> + return __cpu_to_virtio16(tun->flags & IFF_VNET_LE, val);
> +}
> +
> static inline u32 tun_hashfn(u32 rxhash)
> {
> return rxhash & 0x3ff;
> @@ -1053,10 +1063,10 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
> return -EFAULT;
>
> if ((gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
> - gso.csum_start + gso.csum_offset + 2 > gso.hdr_len)
> - gso.hdr_len = gso.csum_start + gso.csum_offset + 2;
> + tun16_to_cpu(tun, gso.csum_start) + tun16_to_cpu(tun,
> gso.csum_offset) + 2 > tun16_to_cpu(tun, gso.hdr_len))
> + gso.hdr_len = cpu_to_tun16(tun, tun16_to_cpu(tun, gso.csum_start)
> + tun16_to_cpu(tun, gso.csum_offset) + 2);
>
> - if (gso.hdr_len > len)
> + if (tun16_to_cpu(tun, gso.hdr_len) > len)
> return -EINVAL;
> offset += tun->vnet_hdr_sz;
> }
> @@ -1064,7 +1074,7 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
> if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) {
> align += NET_IP_ALIGN;
> if (unlikely(len < ETH_HLEN ||
> - (gso.hdr_len && gso.hdr_len < ETH_HLEN)))
> + (gso.hdr_len && tun16_to_cpu(tun, gso.hdr_len) < ETH_HLEN)))
> return -EINVAL;
> }
>
> @@ -1075,7 +1085,7 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
> * enough room for skb expand head in case it is used.
> * The rest of the buffer is mapped from userspace.
> */
> - copylen = gso.hdr_len ? gso.hdr_len : GOODCOPY_LEN;
> + copylen = gso.hdr_len ? tun16_to_cpu(tun, gso.hdr_len) :
> GOODCOPY_LEN;
> if (copylen > good_linear)
> copylen = good_linear;
> linear = copylen;
> @@ -1085,10 +1095,10 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
>
> if (!zerocopy) {
> copylen = len;
> - if (gso.hdr_len > good_linear)
> + if (tun16_to_cpu(tun, gso.hdr_len) > good_linear)
> linear = good_linear;
> else
> - linear = gso.hdr_len;
> + linear = tun16_to_cpu(tun, gso.hdr_len);
> }
>
> skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
> @@ -1115,8 +1125,8 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
> }
>
> if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> - if (!skb_partial_csum_set(skb, gso.csum_start,
> - gso.csum_offset)) {
> + if (!skb_partial_csum_set(skb, tun16_to_cpu(tun, gso.csum_start),
> + tun16_to_cpu(tun, gso.csum_offset))) {
> tun->dev->stats.rx_frame_errors++;
> kfree_skb(skb);
> return -EINVAL;
> @@ -1184,7 +1194,7 @@ static ssize_t tun_get_user(struct tun_struct
> *tun, struct tun_file *tfile,
> if (gso.gso_type & VIRTIO_NET_HDR_GSO_ECN)
> skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
>
> - skb_shinfo(skb)->gso_size = gso.gso_size;
> + skb_shinfo(skb)->gso_size = tun16_to_cpu(tun, gso.gso_size);
> if (skb_shinfo(skb)->gso_size == 0) {
> tun->dev->stats.rx_frame_errors++;
> kfree_skb(skb);
> @@ -1276,8 +1286,8 @@ static ssize_t tun_put_user(struct tun_struct
> *tun,
> struct skb_shared_info *sinfo = skb_shinfo(skb);
>
> /* This is a hint as to how much should be linear. */
> - gso.hdr_len = skb_headlen(skb);
> - gso.gso_size = sinfo->gso_size;
> + gso.hdr_len = cpu_to_tun16(tun, skb_headlen(skb));
> + gso.gso_size = cpu_to_tun16(tun, sinfo->gso_size);
> if (sinfo->gso_type & SKB_GSO_TCPV4)
> gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> else if (sinfo->gso_type & SKB_GSO_TCPV6)
> @@ -1285,12 +1295,12 @@ static ssize_t tun_put_user(struct tun_struct
> *tun,
> else {
> pr_err("unexpected GSO type: "
> "0x%x, gso_size %d, hdr_len %d\n",
> - sinfo->gso_type, gso.gso_size,
> - gso.hdr_len);
> + sinfo->gso_type, tun16_to_cpu(tun, gso.gso_size),
> + tun16_to_cpu(tun, gso.hdr_len));
> print_hex_dump(KERN_ERR, "tun: ",
> DUMP_PREFIX_NONE,
> 16, 1, skb->head,
> - min((int)gso.hdr_len, 64), true);
> + min((int)tun16_to_cpu(tun, gso.hdr_len), 64), true);
> WARN_ON_ONCE(1);
> return -EINVAL;
> }
> @@ -1301,9 +1311,9 @@ static ssize_t tun_put_user(struct tun_struct
> *tun,
>
> if (skb->ip_summed == CHECKSUM_PARTIAL) {
> gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
> - gso.csum_start = skb_checksum_start_offset(skb) +
> - vlan_hlen;
> - gso.csum_offset = skb->csum_offset;
> + gso.csum_start = cpu_to_tun16(tun, skb_checksum_start_offset(skb)
> +
> + vlan_hlen);
> + gso.csum_offset = cpu_to_tun16(tun, skb->csum_offset);
> } else if (skb->ip_summed == CHECKSUM_UNNECESSARY) {
> gso.flags = VIRTIO_NET_HDR_F_DATA_VALID;
> } /* else everything is zero */
> --
> MST
Reviewed-by: Jason Wang <jasowang@redhat.com>
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-11-28 8:17 UTC (permalink / raw)
To: Kevin Zhu
Cc: Alex Strizhevsky, Bjørn Mork, Midge Shaojun Tan,
Enrico Mioso, youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Eli Britstein
In-Reply-To: <54781523.6030600@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 7698 bytes --]
And...why in Windows we have this line?
0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
Is this just padding stuff before the "ncm0" string?
So in particular a simple line needs to be modified in practice in the driver,
or am I totally misguised?
Is our "offset" parameter calculatedsuitably in the driver? Looking at it, any
comment appreciated.
On Fri, 28 Nov 2014, Kevin Zhu wrote:
==Date: Fri, 28 Nov 2014 07:24:49
==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
==To: Alex Strizhevsky <alexxst@gmail.com>, Bjørn Mork <bjorn@mork.no>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>
==Cc: Enrico Mioso <mrkiko.rs@gmail.com>, "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
== Eli Britstein <Eli.Britstein@audiocodes.com>
==Subject: Re: Is this 32-bit NCM?
==
==Hi all,
==
==I'm able to get the following prints with the original huawei_cdc_ncm driver
==from Ubuntu 12.04.3. It keeps on printing. I'm also able to get an IP, but
==no Internet access.
==
==^HCSQ:"WCDMA",64,59,55
==
==^DSFLOWRPT:0000000E,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==OK
==
==^NDISSTAT:1,,,"IPV4"
==
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:00000010,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000012,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000014,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^STIN: 1, 0, 0
==
==
==...
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:0000008C,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:0000008E,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
==Regarding the alignment, I think we have some difference between Windows and
==Linux. The spec says it should satisfy the following constraint.
==
==Offset % wNdpOutDivisor == wNdpOutPayloadRemainder (for OUT datagrams)
==
==It seems the Huawei device take the NDP offset (Ethernet Header Offset) as
==the parameter 'Offset' in the above constraint. And in the Linux capture, it
==does not comply with it.
==
==
==dwNtbInMaxSize=131072 dwNtbOutMaxSize=16384 wNdpOutPayloadRemainder=2
==wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=0 flags=0x1f
==
==Windows:
==
==0000 6e 63 6d 68 10 00 b6 00 7c 00 00 00 5c 00 00 00 ncmh....|...\...
==0010 00 00 00 00 00 00 00 00 00 1e 10 1f 00 00 08 00 ................
==0020 45 00 00 3c 00 9b 00 00 80 01 a0 fe 0a 71 a3 0e E..<.........q..
==0030 ca 6c 21 3c 08 00 4b 4b 00 01 02 10 61 62 63 64 .l!<..KK....abcd
==0040 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 efghijklmnopqrst
==0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
==0060 20 00 00 00 00 00 00 00 00 00 00 00 12 00 00 00 ...............
==0070 4a 00 00 00 00 00 00 00 00 00 00 00 J...........
==
==Linux: (thought it's a 16bit format capture, but it's the same as 32bit,
==regarding the alignment)
==
==0000 4e 43 4d 48 0c 00 40 00 e2 00 0c 00 4e 43 4d 30 NCMH..@.....NCM0
==0010 10 00 00 00 b8 00 2a 00 00 00 00 00 00 00 00 00 ......*.........
==0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0090 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00b0 00 00 00 00 00 00 00 00 ff ff ff ff ff ff 00 1e ................
==00c0 10 1f 00 00 08 06 00 01 08 00 06 04 00 01 00 1e ................
==00d0 10 1f 00 00 0a 71 cc a6 00 00 00 00 00 00 70 50 .....q........pP
==00e0 f8 49 .I
==
==
==Regards,
==Kevin
==On 11/27/2014 08:36 PM, Alex Strizhevsky wrote:
== Adding my colleagues - Eli, Kevin & Midge.
==
==Any ideas are welcome ;)
==
==
==On Thu, Nov 27, 2014 at 12:03 PM, Bjørn Mork <bjorn@mork.no> wrote:
== Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
== > Ok - we can arrive to some ocnclusions regarding the
== E3272.
== > First of all - the modem seems buggy enough to not be
== able to handle requests
== > for different formats. You need to unplug and re-plug
== it, but this is onlyan
== > impression and is reasonable.
== >
== > Then - the modem will accept to ndisdup the connection
== with
== > at^ndisdup=1,1,"internet"
== > but - if we use huawei_cdc_ncm + cdc_ncm we have no flow
== handling messages and
== > the modem stops here.
== > If we use the cdc_ncm 32-bit driver (modified) we get
== lotfs of
== > ^dsflorpt
== > that's how it should be.
== > So I think we can say that something is changing.
== > Then there's the alignment problem you mentioned in your
== previous reply. And
== > this is hard to solve.
== > could you try to help me understand where the problem
== is?
== > I feel like we are very close to the solution but
== something isn't working.
== > Or might be just try to change the 16 bit driver?
==
==If you use a recent version of the driver as a basis, then you
==get the
==CDC NCM NTB parameters in sysfs (if not, then you need to enable
==debugging and look in the log for these values). For example:
==
==bjorn@nemi:~$ grep . /sys/class/net/wwan0/cdc_ncm/*
==/sys/class/net/wwan0/cdc_ncm/bmNtbFormatsSupported:0x0001
==/sys/class/net/wwan0/cdc_ncm/dwNtbInMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/dwNtbOutMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/min_tx_pkt:13824
==/sys/class/net/wwan0/cdc_ncm/rx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_timer_usecs:400
==/sys/class/net/wwan0/cdc_ncm/wNdpInAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpInDivisor:1
==/sys/class/net/wwan0/cdc_ncm/wNdpInPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNdpOutAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpOutDivisor:32
==/sys/class/net/wwan0/cdc_ncm/wNdpOutPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNtbOutMaxDatagrams:32
==
==
==The possible problem I am thinking of is proper handling of the
==wNdp*PayloadRemainder values. See section 3.3.4 "NCM Ethernet
==Frame
==Alignment" in the spec. Which is confusing as hell, but if I
==understand
==it correctly then we are supposed to align the start of the IP
==packets
==(the "payload", _not_ the ethernet frame) to a whole
==wNdp*Divisor number
==as long as the wNdp*PayloadRemainder is 0.
==
==
==Bjørn
==
==
==
==This email and any files transmitted with it are confidential material. They
==are intended solely for the use of the designated individual or entity to
==whom they are addressed. If the reader of this message is not the intended
==recipient, you are hereby notified that any dissemination, use, distribution
==or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the
==sender and delete or destroy any copy of this message
==
^ permalink raw reply
* Re: [PATCH v6 38/46] tun: drop most type defines
From: Jason Wang @ 2014-11-28 8:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel, David Miller, cornelia.huck, rusty, nab, pbonzini,
thuth, dahi, Zhi Yong Wu, Tom Herbert, Ben Hutchings,
Masatake YAMATO, Herbert Xu, Xi Wang, netdev
In-Reply-To: <1417118789-18231-39-git-send-email-mst@redhat.com>
On Fri, Nov 28, 2014 at 4:11 AM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> It's just as easy to use IFF_ flags directly,
> there's no point in adding our own defines.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> drivers/net/tun.c | 62
> +++++++++++++++++++++++++------------------------------
> 1 file changed, 28 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
Patch looks fine, but I don't see the reason that why not squash patch
37
into this.
[..]
^ permalink raw reply
* Re: [PATCH iproute2] ip xfrm: support 64bit kernel and 32bit userspace
From: FengYu LeiDian @ 2014-11-28 8:07 UTC (permalink / raw)
To: Li RongQing; +Cc: netdev
In-Reply-To: <CAJFZqHyHaD7+6fqjmsVh_JeDrto5SmN2D6rZkjWa3axfM0+AiA@mail.gmail.com>
于 2014年11月28日 15:35, Li RongQing 写道:
> On Fri, Nov 28, 2014 at 3:08 PM, FengYu LeiDian
> <fengyuleidian0615@gmail.com> wrote:
>> >于 2014年11月28日 14:58,roy.qing.li@gmail.com 写道:
>>> >>From: Li RongQing<roy.qing.li@gmail.com>
>>> >>
>>> >>The size of struct xfrm_userpolicy_info is 168 bytes for 64bit kernel, and
>>> >>164 bytes for 32bit userspace because of the different alignment. and lead
>>> >>to "ip xfrm" be unable to work.
>>> >>
>>> >>add a pad in struct xfrm_userpolicy_info, and enable it by set
>>> >>KERNEL_64_USERSPACE_32 to y
>> >
>> >
>> >It's not easy to fix it like this.
>> >
>> >Refer:
>> >http://marc.info/?l=linux-netdev&m=139280143513650&w=2
>> >http://thread.gmane.org/gmane.linux.network/157118/focus=157122
>> >
> is it be practical to find all structures which has the issue for
> 64bit kernel and 32 bit
> userspace, and conditionally add a pad for them
Apparently struct xfrm_userpolicy_info is not the only one,
you could try to set SA as well, it breaks also...
Fix those where it hurts, hehe :) Otherwise, the "compat layer"
fix style is good choice.
^ permalink raw reply
* Re: [PATCH v6 37/46] tun: move internal flag defines out of uapi
From: Jason Wang @ 2014-11-28 8:07 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, David Miller,
cornelia.huck-tA70FqPdS9bQT0dZR+AlfA,
rusty-8fk3Idey6ehBDgjK7y7TUQ, nab-IzHhD5pYlfBP7FQvKIMDCQ,
pbonzini-H+wXaHxf7aLQT0dZR+AlfA,
thuth-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8,
dahi-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8, Zhi Yong Wu, Ben Hutchings,
Herbert Xu, Tom Herbert, Masatake YAMATO, Xi Wang,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1417118789-18231-38-git-send-email-mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Fri, Nov 28, 2014 at 4:10 AM, Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
wrote:
> TUN_ flags are internal and never exposed
> to userspace. Any application using it is almost
> certainly buggy.
>
> Move them out to tun.c, we'll remove them in follow-up patches.
>
> Signed-off-by: Michael S. Tsirkin <mst-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> include/uapi/linux/if_tun.h | 16 ++--------
> drivers/net/tun.c | 74
> ++++++++++++++-------------------------------
> 2 files changed, 26 insertions(+), 64 deletions(-)
>
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index e9502dd..277a260 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -22,21 +22,11 @@
>
> /* Read queue size */
> #define TUN_READQ_SIZE 500
> -
> -/* TUN device flags */
> -#define TUN_TUN_DEV 0x0001
> -#define TUN_TAP_DEV 0x0002
> +/* TUN device type flags: deprecated. Use IFF_TUN/IFF_TAP instead. */
> +#define TUN_TUN_DEV IFF_TUN
> +#define TUN_TAP_DEV IFF_TAP
> #define TUN_TYPE_MASK 0x000f
>
> -#define TUN_FASYNC 0x0010
> -#define TUN_NOCHECKSUM 0x0020
> -#define TUN_NO_PI 0x0040
> -/* This flag has no real effect */
> -#define TUN_ONE_QUEUE 0x0080
> -#define TUN_PERSIST 0x0100
> -#define TUN_VNET_HDR 0x0200
> -#define TUN_TAP_MQ 0x0400
> -
> /* Ioctl defines */
> #define TUNSETNOCSUM _IOW('T', 200, int)
> #define TUNSETDEBUG _IOW('T', 201, int)
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 9dd3746..bc89d07 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -103,6 +103,21 @@ do { \
> } while (0)
> #endif
>
> +/* TUN device flags */
> +
> +/* IFF_ATTACH_QUEUE is never stored in device flags,
> + * overload it to mean fasync when stored there.
> + */
> +#define TUN_FASYNC IFF_ATTACH_QUEUE
> +#define TUN_NO_PI IFF_NO_PI
> +/* This flag has no real effect */
> +#define TUN_ONE_QUEUE IFF_ONE_QUEUE
> +#define TUN_PERSIST IFF_PERSIST
> +#define TUN_VNET_HDR IFF_VNET_HDR
> +#define TUN_TAP_MQ IFF_MULTI_QUEUE
> +
> +#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
> + IFF_MULTI_QUEUE)
> #define GOODCOPY_LEN 128
>
> #define FLT_EXACT_COUNT 8
> @@ -1521,32 +1536,7 @@ static struct proto tun_proto = {
>
> static int tun_flags(struct tun_struct *tun)
> {
> - int flags = 0;
> -
> - if (tun->flags & TUN_TUN_DEV)
> - flags |= IFF_TUN;
> - else
> - flags |= IFF_TAP;
> -
> - if (tun->flags & TUN_NO_PI)
> - flags |= IFF_NO_PI;
> -
> - /* This flag has no real effect. We track the value for backwards
> - * compatibility.
> - */
> - if (tun->flags & TUN_ONE_QUEUE)
> - flags |= IFF_ONE_QUEUE;
> -
> - if (tun->flags & TUN_VNET_HDR)
> - flags |= IFF_VNET_HDR;
> -
> - if (tun->flags & TUN_TAP_MQ)
> - flags |= IFF_MULTI_QUEUE;
> -
> - if (tun->flags & TUN_PERSIST)
> - flags |= IFF_PERSIST;
> -
> - return flags;
> + return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN |
> IFF_TAP);
> }
>
> static ssize_t tun_show_flags(struct device *dev, struct
> device_attribute *attr,
> @@ -1706,28 +1696,8 @@ static int tun_set_iff(struct net *net, struct
> file *file, struct ifreq *ifr)
>
> tun_debug(KERN_INFO, tun, "tun_set_iff\n");
>
> - if (ifr->ifr_flags & IFF_NO_PI)
> - tun->flags |= TUN_NO_PI;
> - else
> - tun->flags &= ~TUN_NO_PI;
> -
> - /* This flag has no real effect. We track the value for backwards
> - * compatibility.
> - */
> - if (ifr->ifr_flags & IFF_ONE_QUEUE)
> - tun->flags |= TUN_ONE_QUEUE;
> - else
> - tun->flags &= ~TUN_ONE_QUEUE;
> -
> - if (ifr->ifr_flags & IFF_VNET_HDR)
> - tun->flags |= TUN_VNET_HDR;
> - else
> - tun->flags &= ~TUN_VNET_HDR;
> -
> - if (ifr->ifr_flags & IFF_MULTI_QUEUE)
> - tun->flags |= TUN_TAP_MQ;
> - else
> - tun->flags &= ~TUN_TAP_MQ;
> + tun->flags = (tun->flags & ~TUN_FEATURES) |
> + (ifr->ifr_flags & TUN_FEATURES);
>
> /* Make sure persistent devices do not get stuck in
> * xoff state.
> @@ -1890,9 +1860,11 @@ static long __tun_chr_ioctl(struct file *file,
> unsigned int cmd,
> if (cmd == TUNGETFEATURES) {
> /* Currently this just means: "what IFF flags are valid?".
> * This is needed because we never checked for invalid flags on
> - * TUNSETIFF. */
> - return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
> - IFF_VNET_HDR | IFF_MULTI_QUEUE,
> + * TUNSETIFF. Why do we report IFF_TUN and IFF_TAP which are
> + * not legal for TUNSETIFF here? It's probably a bug, but it
> + * doesn't seem to be worth fixing.
> + */
The question is not very clear. IFF_TUN and IFF_TAP are legal for
ifr.ifr_flags during TUNSETIFF. No?
>
> + return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
> (unsigned int __user*)argp);
> } else if (cmd == TUNSETQUEUE)
> return tun_set_queue(file, &ifr);
> --
> MST
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH iproute2] ip xfrm: support 64bit kernel and 32bit userspace
From: Li RongQing @ 2014-11-28 7:35 UTC (permalink / raw)
To: FengYu LeiDian; +Cc: netdev
In-Reply-To: <54781F55.5040603@gmail.com>
On Fri, Nov 28, 2014 at 3:08 PM, FengYu LeiDian
<fengyuleidian0615@gmail.com> wrote:
> 于 2014年11月28日 14:58, roy.qing.li@gmail.com 写道:
>> From: Li RongQing<roy.qing.li@gmail.com>
>>
>> The size of struct xfrm_userpolicy_info is 168 bytes for 64bit kernel, and
>> 164 bytes for 32bit userspace because of the different alignment. and lead
>> to "ip xfrm" be unable to work.
>>
>> add a pad in struct xfrm_userpolicy_info, and enable it by set
>> KERNEL_64_USERSPACE_32 to y
>
>
> It's not easy to fix it like this.
>
> Refer:
> http://marc.info/?l=linux-netdev&m=139280143513650&w=2
> http://thread.gmane.org/gmane.linux.network/157118/focus=157122
>
is it be practical to find all structures which has the issue for
64bit kernel and 32 bit
userspace, and conditionally add a pad for them
-Roy
^ permalink raw reply
* Re: [PATCH iproute2] ip xfrm: support 64bit kernel and 32bit userspace
From: FengYu LeiDian @ 2014-11-28 7:08 UTC (permalink / raw)
To: roy.qing.li; +Cc: netdev
In-Reply-To: <1417157936-5522-1-git-send-email-roy.qing.li@gmail.com>
于 2014年11月28日 14:58, roy.qing.li@gmail.com 写道:
> From: Li RongQing<roy.qing.li@gmail.com>
>
> The size of struct xfrm_userpolicy_info is 168 bytes for 64bit kernel, and
> 164 bytes for 32bit userspace because of the different alignment. and lead
> to "ip xfrm" be unable to work.
>
> add a pad in struct xfrm_userpolicy_info, and enable it by set
> KERNEL_64_USERSPACE_32 to y
It's not easy to fix it like this.
Refer:
http://marc.info/?l=linux-netdev&m=139280143513650&w=2
http://thread.gmane.org/gmane.linux.network/157118/focus=157122
^ permalink raw reply
* Re: [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Jason Wang @ 2014-11-28 7:02 UTC (permalink / raw)
To: Fan Du; +Cc: netdev, davem, fw, Fan Du
In-Reply-To: <1417156385-18276-1-git-send-email-fan.du@intel.com>
On Fri, Nov 28, 2014 at 2:33 PM, Fan Du <fan.du@intel.com> wrote:
> Test scenario: two KVM guests sitting in different
> hosts communicate to each other with a vxlan tunnel.
>
> All interface MTU is default 1500 Bytes, from guest point
> of view, its skb gso_size could be as bigger as 1448Bytes,
> however after guest skb goes through vxlan encapuslation,
> individual segments length of a gso packet could exceed
> physical NIC MTU 1500, which will be lost at recevier side.
>
> So it's possible in virtualized environment, locally created
> skb len after encapslation could be bigger than underlayer
> MTU. In such case, it's reasonable to do GSO first,
> then fragment any packet bigger than MTU as possible.
>
> +---------------+ TX RX +---------------+
> | KVM Guest | -> ... -> | KVM Guest |
> +-+-----------+-+ +-+-----------+-+
> |Qemu/VirtIO| |Qemu/VirtIO|
> +-----------+ +-----------+
> | |
> v tap0 tap0 v
> +-----------+ +-----------+
> | ovs bridge| | ovs bridge|
> +-----------+ +-----------+
> | vxlan vxlan |
> v v
> +-----------+ +-----------+
> | NIC | <------> | NIC |
> +-----------+ +-----------+
>
> Steps to reproduce:
> 1. Using kernel builtin openvswitch module to setup ovs bridge.
> 2. Runing iperf without -M, communication will stuck.
Is this issue specific to ovs or ipv4? Path MTU discovery should
help in this case I believe.
>
>
> Signed-off-by: Fan Du <fan.du@intel.com>
> ---
> net/ipv4/ip_output.c | 7 ++++---
> 1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index bc6471d..558b5f8 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -217,9 +217,10 @@ static int ip_finish_output_gso(struct sk_buff
> *skb)
> struct sk_buff *segs;
> int ret = 0;
>
> - /* common case: locally created skb or seglen is <= mtu */
> - if (((IPCB(skb)->flags & IPSKB_FORWARDED) == 0) ||
> - skb_gso_network_seglen(skb) <= ip_skb_dst_mtu(skb))
> + /* Both locally created skb and forwarded skb could exceed
> + * MTU size, so make a unified rule for them all.
> + */
> + if (skb_gso_network_seglen(skb) <= ip_skb_dst_mtu(skb))
> return ip_finish_output2(skb);
>
> /* Slowpath - GSO segment length is exceeding the dst MTU.
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH iproute2] ip xfrm: support 64bit kernel and 32bit userspace
From: roy.qing.li @ 2014-11-28 6:58 UTC (permalink / raw)
To: netdev
From: Li RongQing <roy.qing.li@gmail.com>
The size of struct xfrm_userpolicy_info is 168 bytes for 64bit kernel, and
164 bytes for 32bit userspace because of the different alignment. and lead
to "ip xfrm" be unable to work.
add a pad in struct xfrm_userpolicy_info, and enable it by set
KERNEL_64_USERSPACE_32 to y
Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
Makefile | 4 ++++
include/linux/xfrm.h | 3 +++
2 files changed, 7 insertions(+)
diff --git a/Makefile b/Makefile
index 9dbb29f..3cd0d67 100644
--- a/Makefile
+++ b/Makefile
@@ -18,6 +18,10 @@ ifneq ($(SHARED_LIBS),y)
DEFINES+= -DNO_SHARED_LIBS
endif
+ifeq ($(KERNEL_64_USERSPACE_32),y)
+DEFINES+= -DKNL_64_US_32
+endif
+
DEFINES+=-DCONFDIR=\"$(CONFDIR)\"
#options for decnet
diff --git a/include/linux/xfrm.h b/include/linux/xfrm.h
index fa2ecb2..009510c 100644
--- a/include/linux/xfrm.h
+++ b/include/linux/xfrm.h
@@ -407,6 +407,9 @@ struct xfrm_userpolicy_info {
/* Automatically expand selector to include matching ICMP payloads. */
#define XFRM_POLICY_ICMP 2
__u8 share;
+#ifdef KNL_64_US_32
+ int pad;
+#endif
};
struct xfrm_userpolicy_id {
--
2.1.0
^ permalink raw reply related
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-11-28 6:40 UTC (permalink / raw)
To: Kevin Zhu
Cc: Alex Strizhevsky, Bjørn Mork, Midge Shaojun Tan,
youtux@gmail.com, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Eli Britstein
In-Reply-To: <54781523.6030600@audiocodes.com>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 7805 bytes --]
Ok.
On Fri, 28 Nov 2014, Kevin Zhu wrote:
==Date: Fri, 28 Nov 2014 07:24:49
==From: Kevin Zhu <Mingying.Zhu@audiocodes.com>
==To: Alex Strizhevsky <alexxst@gmail.com>, Bjørn Mork <bjorn@mork.no>,
== Midge Shaojun Tan <ShaojunMidge.Tan@audiocodes.com>
==Cc: Enrico Mioso <mrkiko.rs@gmail.com>, "youtux@gmail.com" <youtux@gmail.com>,
== "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
== "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
== Eli Britstein <Eli.Britstein@audiocodes.com>
==Subject: Re: Is this 32-bit NCM?
==
==Hi all,
==
==I'm able to get the following prints with the original huawei_cdc_ncm driver
==from Ubuntu 12.04.3. It keeps on printing. I'm also able to get an IP, but
==no Internet access.
==
==^HCSQ:"WCDMA",64,59,55
==
==^DSFLOWRPT:0000000E,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==OK
==
==^NDISSTAT:1,,,"IPV4"
==
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:00000010,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000012,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:00000014,00000000,00000000,0000000000000000,0000000000000000,000
==00000,00000000
==
==^STIN: 1, 0, 0
==
==
==...
==^STIN: 1, 0, 0
==
==^DSFLOWRPT:0000008C,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
==^DSFLOWRPT:0000008E,00000000,00000000,0000000000000B36,0000000000000000,000
==00000,00000000
==
This is a clear indication of success from your side - in the sense that you
where able to ask the modem to connect, and get it connected.
Any time you see messages regarding the flow, then it's ok.
Thank you for your great job guys.
Now - can you try to patch the cdc_ncm.c 16 bit driver to respect this
alignment and see what's wrong with it?
I will also take a look at it, but I am not so sure to succeed in doing so. But
I'll re-read all when arrived in university.
Enrico
==Regarding the alignment, I think we have some difference between Windows and
==Linux. The spec says it should satisfy the following constraint.
==
==Offset % wNdpOutDivisor == wNdpOutPayloadRemainder (for OUT datagrams)
==
==It seems the Huawei device take the NDP offset (Ethernet Header Offset) as
==the parameter 'Offset' in the above constraint. And in the Linux capture, it
==does not comply with it.
==
==
==dwNtbInMaxSize=131072 dwNtbOutMaxSize=16384 wNdpOutPayloadRemainder=2
==wNdpOutDivisor=4 wNdpOutAlignment=4 wNtbOutMaxDatagrams=0 flags=0x1f
==
==Windows:
==
==0000 6e 63 6d 68 10 00 b6 00 7c 00 00 00 5c 00 00 00 ncmh....|...\...
==0010 00 00 00 00 00 00 00 00 00 1e 10 1f 00 00 08 00 ................
==0020 45 00 00 3c 00 9b 00 00 80 01 a0 fe 0a 71 a3 0e E..<.........q..
==0030 ca 6c 21 3c 08 00 4b 4b 00 01 02 10 61 62 63 64 .l!<..KK....abcd
==0040 65 66 67 68 69 6a 6b 6c 6d 6e 6f 70 71 72 73 74 efghijklmnopqrst
==0050 75 76 77 61 62 63 64 65 66 67 68 69 6e 63 6d 30 uvwabcdefghincm0
==0060 20 00 00 00 00 00 00 00 00 00 00 00 12 00 00 00 ...............
==0070 4a 00 00 00 00 00 00 00 00 00 00 00 J...........
==
==Linux: (thought it's a 16bit format capture, but it's the same as 32bit,
==regarding the alignment)
==
==0000 4e 43 4d 48 0c 00 40 00 e2 00 0c 00 4e 43 4d 30 NCMH..@.....NCM0
==0010 10 00 00 00 b8 00 2a 00 00 00 00 00 00 00 00 00 ......*.........
==0020 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0030 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0040 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0070 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0080 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==0090 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00a0 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
==00b0 00 00 00 00 00 00 00 00 ff ff ff ff ff ff 00 1e ................
==00c0 10 1f 00 00 08 06 00 01 08 00 06 04 00 01 00 1e ................
==00d0 10 1f 00 00 0a 71 cc a6 00 00 00 00 00 00 70 50 .....q........pP
==00e0 f8 49 .I
==
==
==Regards,
==Kevin
==On 11/27/2014 08:36 PM, Alex Strizhevsky wrote:
== Adding my colleagues - Eli, Kevin & Midge.
==
==Any ideas are welcome ;)
==
==
==On Thu, Nov 27, 2014 at 12:03 PM, Bjørn Mork <bjorn@mork.no> wrote:
== Enrico Mioso <mrkiko.rs@gmail.com> writes:
==
== > Ok - we can arrive to some ocnclusions regarding the
== E3272.
== > First of all - the modem seems buggy enough to not be
== able to handle requests
== > for different formats. You need to unplug and re-plug
== it, but this is onlyan
== > impression and is reasonable.
== >
== > Then - the modem will accept to ndisdup the connection
== with
== > at^ndisdup=1,1,"internet"
== > but - if we use huawei_cdc_ncm + cdc_ncm we have no flow
== handling messages and
== > the modem stops here.
== > If we use the cdc_ncm 32-bit driver (modified) we get
== lotfs of
== > ^dsflorpt
== > that's how it should be.
== > So I think we can say that something is changing.
== > Then there's the alignment problem you mentioned in your
== previous reply. And
== > this is hard to solve.
== > could you try to help me understand where the problem
== is?
== > I feel like we are very close to the solution but
== something isn't working.
== > Or might be just try to change the 16 bit driver?
==
==If you use a recent version of the driver as a basis, then you
==get the
==CDC NCM NTB parameters in sysfs (if not, then you need to enable
==debugging and look in the log for these values). For example:
==
==bjorn@nemi:~$ grep . /sys/class/net/wwan0/cdc_ncm/*
==/sys/class/net/wwan0/cdc_ncm/bmNtbFormatsSupported:0x0001
==/sys/class/net/wwan0/cdc_ncm/dwNtbInMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/dwNtbOutMaxSize:15360
==/sys/class/net/wwan0/cdc_ncm/min_tx_pkt:13824
==/sys/class/net/wwan0/cdc_ncm/rx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_max:15360
==/sys/class/net/wwan0/cdc_ncm/tx_timer_usecs:400
==/sys/class/net/wwan0/cdc_ncm/wNdpInAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpInDivisor:1
==/sys/class/net/wwan0/cdc_ncm/wNdpInPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNdpOutAlignment:4
==/sys/class/net/wwan0/cdc_ncm/wNdpOutDivisor:32
==/sys/class/net/wwan0/cdc_ncm/wNdpOutPayloadRemainder:0
==/sys/class/net/wwan0/cdc_ncm/wNtbOutMaxDatagrams:32
==
==
==The possible problem I am thinking of is proper handling of the
==wNdp*PayloadRemainder values. See section 3.3.4 "NCM Ethernet
==Frame
==Alignment" in the spec. Which is confusing as hell, but if I
==understand
==it correctly then we are supposed to align the start of the IP
==packets
==(the "payload", _not_ the ethernet frame) to a whole
==wNdp*Divisor number
==as long as the wNdp*PayloadRemainder is 0.
==
==
==Bjørn
==
==
==
==This email and any files transmitted with it are confidential material. They
==are intended solely for the use of the designated individual or entity to
==whom they are addressed. If the reader of this message is not the intended
==recipient, you are hereby notified that any dissemination, use, distribution
==or copying of this communication is strictly prohibited and may be unlawful.
==
==If you have received this email in error please immediately notify the
==sender and delete or destroy any copy of this message
==
^ permalink raw reply
* [PATCH net] gso: do GSO for local skb with size bigger than MTU
From: Fan Du @ 2014-11-28 6:33 UTC (permalink / raw)
To: netdev; +Cc: davem, fw, Fan Du
Test scenario: two KVM guests sitting in different
hosts communicate to each other with a vxlan tunnel.
All interface MTU is default 1500 Bytes, from guest point
of view, its skb gso_size could be as bigger as 1448Bytes,
however after guest skb goes through vxlan encapuslation,
individual segments length of a gso packet could exceed
physical NIC MTU 1500, which will be lost at recevier side.
So it's possible in virtualized environment, locally created
skb len after encapslation could be bigger than underlayer
MTU. In such case, it's reasonable to do GSO first,
then fragment any packet bigger than MTU as possible.
+---------------+ TX RX +---------------+
| KVM Guest | -> ... -> | KVM Guest |
+-+-----------+-+ +-+-----------+-+
|Qemu/VirtIO| |Qemu/VirtIO|
+-----------+ +-----------+
| |
v tap0 tap0 v
+-----------+ +-----------+
| ovs bridge| | ovs bridge|
+-----------+ +-----------+
| vxlan vxlan |
v v
+-----------+ +-----------+
| NIC | <------> | NIC |
+-----------+ +-----------+
Steps to reproduce:
1. Using kernel builtin openvswitch module to setup ovs bridge.
2. Runing iperf without -M, communication will stuck.
Signed-off-by: Fan Du <fan.du@intel.com>
---
net/ipv4/ip_output.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index bc6471d..558b5f8 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -217,9 +217,10 @@ static int ip_finish_output_gso(struct sk_buff *skb)
struct sk_buff *segs;
int ret = 0;
- /* common case: locally created skb or seglen is <= mtu */
- if (((IPCB(skb)->flags & IPSKB_FORWARDED) == 0) ||
- skb_gso_network_seglen(skb) <= ip_skb_dst_mtu(skb))
+ /* Both locally created skb and forwarded skb could exceed
+ * MTU size, so make a unified rule for them all.
+ */
+ if (skb_gso_network_seglen(skb) <= ip_skb_dst_mtu(skb))
return ip_finish_output2(skb);
/* Slowpath - GSO segment length is exceeding the dst MTU.
--
1.7.1
^ permalink raw reply related
* [WTF?] random test in netlink_sendmsg()
From: Al Viro @ 2014-11-28 6:23 UTC (permalink / raw)
To: David Miller; +Cc: Patrick McHardy, netdev
In netlink_sendmsg() we have the following:
if (netlink_tx_is_mmaped(sk) &&
msg->msg_iov->iov_base == NULL) {
err = netlink_mmap_sendmsg(sk, msg, dst_portid, dst_group,
siocb);
goto out;
}
Now, suppose sendmsg(2) is called with msg.msg_iovlen == 0. We'll have
->msg_iov in kernel-side copy pointing at the uninitialized array in
stack frame of ___sys_sendmsg() - neither new nor old code touches elements
past the first msg_iovlen ones. So in that case it checks if an
uninitialized word on stack is zero.
What is that check trying to do? Is that simply missing
"(msg->msg_iovlen > 0) &&"? And why on the Earth didn't it simply use
zero msg_iovlen as the indicator, instead of messing with iovec contents?
Obviously too late to change, but... ouch.
Patrick, it had been that way since your commit last year ("netlink: implement
memory mapped sendmsg()"); could you explain what's the intended ABI?
Incidentally, WTF is "atomic_read(&nlk->mapped) > 1" part of check in
netlink_mmap_sendmsg() trying to achieve? AFAICS, ->mapped tries to
keep track of the number of VMAs, right? If so, it's bloody pointless -
one can have memory accessible in more than one process without any
extra VMAs. Just clone(2) with CLONE_VM. Voila - child shares the
entire address space. No extra VMAs or calls of ->open() in sight...
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-11-28 6:23 UTC (permalink / raw)
To: Alex Strizhevsky
Cc: Bjørn Mork, ShaojunMidge.Tan, Mingying.Zhu, youtux,
linux-usb, netdev, Eli.Britstein
In-Reply-To: <CAPChA0cLuvRpYTYuFoi3bewqASgf_oZHSz5yQQ8JYm96dZ4-TQ@mail.gmail.com>
And... a precisation: when I talk about the 32-bit mode driver, I am not
referring to the windows one, but to the Linux one I and Alessio "implemented",
modifying the standard cdc_ncm.c driver.
This version of the driver should be available here:
On Thu, 27 Nov 2014, Alex Strizhevsky wrote:
http://www.gstorm.eu/cdc_ncm.c
And you might compile it with a kernel >= 3.16.
Overmore, if this driver is able to work with E3272, you might compare it with
the Windows driver in E3272 and then compare differences when working with
E3276.
Please Kevin, Midge, and anyone who want to help: do your best. I can't do much
because I don't have non-working device and a Windows installation ready at
hand; but the real problem is the devices.
But for me this is a personal challenge - a matter of heart; and solving this
problem means solving lots of them in future maybe.
So - waiting for any data and consideration.
If you need anything by my side - you know where you can find me:
- here
- or use this mail address as google talk messaging + google plus; and this is
true even for who is reading linux-usb and netdev mailing lists.
Enrico
^ permalink raw reply
* Re: [PATCH rfc] packet: zerocopy packet_snd
From: Jason Wang @ 2014-11-28 6:21 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Willem de Bruijn, Network Development, David Miller, Eric Dumazet,
Daniel Borkmann
In-Reply-To: <20141127104445.GA8961@redhat.com>
On Thu, Nov 27, 2014 at 6:44 PM, Michael S. Tsirkin <mst@redhat.com>
wrote:
> On Thu, Nov 27, 2014 at 09:18:12AM +0008, Jason Wang wrote:
>>
>>
>> On Thu, Nov 27, 2014 at 5:17 AM, Michael S. Tsirkin
>> <mst@redhat.com> wrote:
>> >On Wed, Nov 26, 2014 at 02:59:34PM -0500, Willem de Bruijn wrote:
>> >> > The main problem with zero copy ATM is with queueing
>> disciplines
>> >> > which might keep the socket around essentially forever.
>> >> > The case was described here:
>> >> > https://lkml.org/lkml/2014/1/17/105
>> >> > and of course this will make it more serious now that
>> >> > more applications will be able to do this, so
>> >> > chances that an administrator enables this
>> >> > are higher.
>> >> The denial of service issue raised there, that a single queue can
>> >> block an entire virtio-net device, is less problematic in the
>> case of
>> >> packet sockets. A socket can run out of sk_wmem_alloc, but a
>> prudent
>> >> application can increase the limit or use separate sockets for
>> >> separate flows.
>> >
>> >Socket per flow? Maybe just use TCP then? increasing the limit
>> >sounds like a wrong solution, it hurts security.
>> >
>> >> > One possible solution is some kind of timer orphaning frags
>> >> > for skbs that have been around for too long.
>> >> Perhaps this can be approximated without an explicit timer by
>> calling
>> >> skb_copy_ubufs on enqueue whenever qlen exceeds a threshold
>> value?
>> >
>> >Hard to say. Will have to see that patch to judge how robust this
>> is.
>>
>> This could not work, consider if the threshold is greater than
>> vring size
>> or vhost_net pending limit, transmission may still be blocked.
>
> Well, application can e.g. just switch to non zero copy after
> reaching a specific number of requests.
Yes but only works if user are ok for out of order completion.
>
> I think the real problem isn't reaching the queue full
> condition, it's the fact a specific buffer might never
> get freed. This API isn't half as useful as it could be
> if applications had a way to force the memory
> to be reclaimed.
>
Agree.
>
> And actually, I see a way for applications to reclaim the memory:
> application could invoke something like MADV_SOFT_OFFLINE on the
> memory
> submitted for zero copy transmit, to invalidate PTEs, and make next
> access fault new pages in.
> If dedicated memory is used for packets, you could even use
> MADV_DONTNEED - but this doesn't work in many cases, certainly
> not for virtualization type workloads.
>
> Playting with PTEs needs to invalidate the TLB so it is not fast,
> but it does not need to be: we are talking about ability to close the
> socket, which should be rare.
>
> For example, an application/hypervisor can detect a timeout when a
> packet is not transmitted within a predefined time period, and trigger
> such reclaim.
> Making this period shorter than network watchdog timer of the VM
> will ensure that watchdog does not trigger within VM.
> Alternatively, VM network watchdog could trigger this reclaim
> in order to recover packet memory.
Doing such in hypervisor seems better. It could reduce the possible
guest triggered behavior.
But this just can fix the transmission stuck in guest, host socket
still need to wait for the packet to be sent by host?
>
>
> With this idea, if application merely reads memory, we incur a lot of
> overhead with pagefaults. So maybe a new call to enable COW for a
> range
> of pages would be a good idea.
>
Not very clear, doesn't COW still depends on pagefault to work?
>
> We'd have to make sure whatever's used for reclaim works for
> a wide range of memory types: mmap-ed file, hugetlbfs, anonymous
> memory.
>
>
> Thoughts?
>
> --
> MST
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Is this 32-bit NCM?
From: Enrico Mioso @ 2014-11-28 5:59 UTC (permalink / raw)
To: Alex Strizhevsky
Cc: Bjørn Mork, ShaojunMidge.Tan-6C2+4RG2qWF0ubjbjo6WXg,
Mingying.Zhu-6C2+4RG2qWF0ubjbjo6WXg,
youtux-Re5JQEeQqe8AvxtiuMwx3w, linux-usb-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
Eli.Britstein-6C2+4RG2qWF0ubjbjo6WXg
In-Reply-To: <CAPChA0cLuvRpYTYuFoi3bewqASgf_oZHSz5yQQ8JYm96dZ4-TQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello to everyone.
Can you guys try to compare what happens withe standard, and 32-bit version of
the driver, with the E3276, so that we see what happens and what are the
differences?
And it would be very nice if you look what happens with the standard 16-bit
driver to confirm what Bjorn suspected regarding the alignment problems: this
morning I will take a lookt at the align_tail function (it's called something
like that) in cdc_ncm.c (standard version).
Bjorn and all: do you think it's ok if I keep you all + mailing lists in CC or
would you like me to do something different? Please comment on this if any
change is required.
Taking a shower right now,
Enrico
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net] bpf: x86: fix epilogue generation for eBPF programs
From: Alexei Starovoitov @ 2014-11-28 5:55 UTC (permalink / raw)
To: Daniel Borkmann
Cc: David S. Miller, Zi Shen Lim, Eric Dumazet, H. Peter Anvin,
Thomas Gleixner, Ingo Molnar, Network Development, LKML
In-Reply-To: <5476F471.80500@redhat.com>
On Thu, Nov 27, 2014 at 1:52 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> On 11/27/2014 06:02 AM, Alexei Starovoitov wrote:
>>
>> classic BPF has a restriction that last insn is always BPF_RET.
>> eBPF doesn't have BPF_RET instruction and this restriction.
>> It has BPF_EXIT insn which can appear anywhere in the program
>> one or more times and it doesn't have to be last insn.
>> Fix eBPF JIT to emit epilogue when first BPF_EXIT is seen
>> and all other BPF_EXIT instructions will be emitted as jump.
>>
>> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
>> ---
>> Note, this bug is applicable only to native eBPF programs
>> which first were introduced in 3.18, so no need to send it
>> to stable and therefore no 'Fixes' tag.
>
>
> Btw, even if it's not sent to -stable, a 'Fixes:' tag is useful
> information for backporting and regression tracking, preferably
> always mentioned where it can clearly be identified.
Well I didn't mention it, as I said, because I don't think it
needs backporting. Otherwise with the tag the tools might
pick it up automatically? Just a guess.
Anyway:
Fixes: 622582786c9e ("net: filter: x86: internal BPF JIT")
>> arm64 JIT has the same problem, but the fix is not as trivial,
>> so will be done as separate patch.
>>
>> Since 3.18 can only load eBPF programs and cannot execute them,
>> this patch can even be done in net-next only, but I think it's worth
>> to apply it to 3.18(net), so that JITed output for native eBPF
>> programs is correct when bpf syscall loads it with
>> net.core.bpf_jit_enable=2
>
>
> Yes, sounds good to me, the condition insn_cnt - 1 is still held
> with BPF to eBPF transformations.
Correct. That's what I meant that prior to 3.18 it's not needed.
and 'insn_cnt - 1' condition will still hold for classic in the future.
>> arch/x86/net/bpf_jit_comp.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
>> index 3f62734..7e90244 100644
>> --- a/arch/x86/net/bpf_jit_comp.c
>> +++ b/arch/x86/net/bpf_jit_comp.c
>> @@ -178,7 +178,7 @@ static void jit_fill_hole(void *area, unsigned int
>> size)
>> }
>>
>> struct jit_context {
>> - unsigned int cleanup_addr; /* epilogue code offset */
>> + int cleanup_addr; /* epilogue code offset */
>
>
> Why this type change here? This seems a bit out of context (I would
> have expected a mention of this in the commit message, otherwise).
Ok. Will respin with updated commit msg.
The reason for signed is the following:
jmp offset to epilogue is computed as:
jmp_offset = ctx->cleanup_addr - addrs[i]
when cleanup_addr was always last insn it wasn't a problem,
since result of subtraction was positive.
Now, since epilogue will be in the middle of JITed
code the jmps to epilogue may be negative, so
signed int is need to do the math correctly.
In other words, it should be:
(long long) ((int)20 - (int)30)
instead of:
(long long) ((unsigned int)20 - (int)30)
^ permalink raw reply
* Re: ipv4: Use standard iovec primitive in raw_probe_proto_opt
From: Al Viro @ 2014-11-28 5:14 UTC (permalink / raw)
To: Jon Maloy
Cc: Herbert Xu, David Miller, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, bcrl@kvack.org, Masahide Nakamura,
Hideaki YOSHIFUJI
In-Reply-To: <20141106221608.GA7996@ZenIV.linux.org.uk>
On Thu, Nov 06, 2014 at 10:16:08PM +0000, Al Viro wrote:
> On Thu, Nov 06, 2014 at 09:55:31AM +0000, Jon Maloy wrote:
> > > Point, but that might very well be a pattern to watch for - there's at least one
> > > more instance in TIPC (also not exploitable, according to TIPC folks) and such
> >
> > I don't recall this, and I can't see where it would be either. Can you please
> > point to where it is?
>
> The same dest_name_check() thing. This
> if (copy_from_user(&hdr, m->msg_iov[0].iov_base, sizeof(hdr)))
> return -EFAULT;
> if ((ntohs(hdr.tcm_type) & 0xC000) && (!capable(CAP_NET_ADMIN)))
> return -EACCES;
> is easily bypassed. Suppose you want to send a packet with these two
> bits in ->tcm_type not being 00, and you don't have CAP_NET_ADMIN.
> Not a problem - spawn two threads sharing memory, have one trying to
> call sendmsg() while another keeps flipping these two bits. Sooner
> of later you'll get the timing right and have these bits observed as 00
> in dest_name_check() and 11 when it comes to memcpy_fromiovecend() actually
> copying the whole thing. And considering that the interval between those
> two is much longer than the loop in the second thread would take on
> each iteration, I'd expect the odds around 25% per attempted sendmsg().
>
> IOW, this test is either pointless and can be removed completely, or there's
> an exploitable race. As far as I understand from your replies both back then
> and in another branch of this thread, it's the former and the proper fix is
> to remove at least that part of dest_name_check(). So this case is also
> not something exploitable, but it certainly matches the same pattern.
>
> My point was simply that this pattern is worth watching for - recurrent bug
> classes like that have a good chance to spawn an instance that will be
> exploitable.
Ping? Can we simply remove dest_name_check() completely? That's one of the
few remaining obstacles to making ->sendmsg() iov_iter-clean. For now I'm
simply commenting its call out in tipc_sendmsg(); if it _is_ needed for
anything, we'll need to get rid of that double copying from userland. I can
do that, but my impression from your comments back in April is that you
planned to removed the damn check anyway.
Another question: in tipc_send_stream() we have
mtu = tsk->max_pkt;
send = min_t(uint, dsz - sent, TIPC_MAX_USER_MSG_SIZE);
__skb_queue_head_init(&head);
rc = tipc_msg_build(mhdr, m, sent, send, mtu, &head);
if (unlikely(rc < 0))
goto exit;
do {
if (likely(!tsk_conn_cong(tsk))) {
rc = tipc_link_xmit(&head, dnode, ref);
if (likely(!rc)) {
tsk->sent_unacked++;
sent += send;
if (sent == dsz)
break;
goto next;
}
if (rc == -EMSGSIZE) {
tsk->max_pkt = tipc_node_get_mtu(dnode, ref);
goto next;
}
How can it hit that EMSGSIZE? AFAICS, it can come only from
int __tipc_link_xmit(struct tipc_link *link, struct sk_buff_head *list)
{
struct tipc_msg *msg = buf_msg(skb_peek(list));
uint psz = msg_size(msg);
...
uint mtu = link->max_pkt;
...
/* Has valid packet limit been used ? */
if (unlikely(psz > mtu)) {
__skb_queue_purge(list);
return -EMSGSIZE;
}
and msg_size() is basically the bits copied into skb by tipc_msg_build() and
set by msg_set_size() in there. And unless I'm seriously misreading that
function, it can't be more than pktmax argument, i.e. mtu. So unless something
manages to crap into our skb or change mtu right under us, it shouldn't be
possible. And mtu (i.e. ->max_pkt) ought to be protected by lock_sock() there.
What's going on there?
^ permalink raw reply
* Re: [PATCH][v3] mdio-mux-gpio: Use GPIO descriptor interface and new gpiod_set_array function
From: Alexandre Courbot @ 2014-11-28 4:30 UTC (permalink / raw)
To: Rojhalat Ibrahim
Cc: linux-gpio@vger.kernel.org, netdev, Linus Walleij,
Alexandre Courbot, David Miller, Florian Fainelli, David Daney
In-Reply-To: <1581158.UhESICdgeM@pcimr>
On Tue, Nov 25, 2014 at 10:57 PM, Rojhalat Ibrahim <imr@rtschenk.de> wrote:
> Convert mdio-mux-gpio to the GPIO descriptor interface and use the new
> gpiod_set_array function to set all output signals simultaneously.
>
> Signed-off-by: Rojhalat Ibrahim <imr@rtschenk.de>
> Acked-by: David S. Miller <davem@davemloft.net>
> --
> This patch depends on the gpiod_set_array function, which is available in
> the linux-gpio devel tree.
>
> Change log:
> v3: remove unnecessary braces
> v2: fix gpiod_get_index usage
>
> drivers/net/phy/mdio-mux-gpio.c | 37 +++++++++++++------------------------
> 1 file changed, 13 insertions(+), 24 deletions(-)
Looks all good to me! Nice use of this new interface, leading to
simpler client code.
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
^ permalink raw reply
* Re: Multiple DSA switch on shared MII
From: Florian Fainelli @ 2014-11-28 12:21 UTC (permalink / raw)
To: Rajib Karmakar; +Cc: netdev
In-Reply-To: <CAOi_9k9h4N-uPf7shapytV+rCtHFWiVco26hCVjTMN=00o8KAg@mail.gmail.com>
Le 26/11/2014 23:18, Rajib Karmakar a écrit :
> This could be a naive question; but can I add all LAN and WAN
> interfaces in a single DSA switch (one cpu port, one netdev)?
One DSA switch is backed by a single netdev, I would rather register two
dsa switches, even if they actually map to the same physical switch,
rather than hacking around a single instance to work with multiple
network devices.
Thanks
--
Florian
^ permalink raw reply
* Charity Work
From: luv2charitys @ 2014-11-28 2:20 UTC (permalink / raw)
To: netdev
Hello,this is Mr Paul N,i sent you an email on charity work but i am yet to hear fom you,do reply with this code CHA-2015 to my email address paulcharity@qq.com i Look forward to hearing from you this time,God bless Brother Paul
^ permalink raw reply
* Re: 3.12.33 Bug with ipvs
From: Smart Weblications GmbH - Florian Wiessner @ 2014-11-28 2:02 UTC (permalink / raw)
To: Julian Anastasov; +Cc: netdev
In-Reply-To: <alpine.LFD.2.11.1411270945240.7068@ja.home.ssi.bg>
Hi,
Am 27.11.2014 09:08, schrieb Julian Anastasov:
>
> Hello,
>
> On Wed, 26 Nov 2014, Smart Weblications GmbH - Florian Wiessner wrote:
>
>> Hi netdev,
>>
>> On 3.12.33 i see this every 3 hours or so on a box with ip_vs running with a
>> setup which made no problems on 3.10.40. Could someone give me hints how to
>> debug this? It seems to happen instantly, when i add ip_vs_ftp and have some nat
>> rules. Setup is like this:
>>
>
>> [13230.431740] RIP [<ffffffff814ff2fc>] xfrm_selector_match+0x25/0x2f6
>> [13230.431772] RSP <ffff88083fd83a68>
>> [13230.431795] CR2: 00000000000600d0
>> [13230.432240] ---[ end trace 103912aa204977dc ]---
>>
>> node01:/ocfs2/usr/src/linux-3.12.33/scripts# ./decodecode </tmp/oops.log
>> [13230.431464] Code: 5d 41 5e 41 5f c3 41 55 66 83 fa 02 41 54 55 48 89 fd 53 48
>> 89 f3 41 50 74 11 31 c0 66 83 fa 0a 0f 85 ce 02 00 00 e9 fd 00 00 00 <0f> b6 47
>> 2a 8b 17 8b 76 18 84 c0 74 1a b9 20 00 00 00 31 f2 29
>> All code
>> ========
>> 0: 5d pop %rbp
>> 1: 41 5e pop %r14
>> 3: 41 5f pop %r15
>> 5: c3 retq
>> 6: 41 55 push %r13
>> 8: 66 83 fa 02 cmp $0x2,%dx
>> c: 41 54 push %r12
>> e: 55 push %rbp
>> f: 48 89 fd mov %rdi,%rbp
>> 12: 53 push %rbx
>> 13: 48 89 f3 mov %rsi,%rbx
>> 16: 41 50 push %r8
>> 18: 74 11 je 0x2b
>> 1a: 31 c0 xor %eax,%eax
>> 1c: 66 83 fa 0a cmp $0xa,%dx
>> 20: 0f 85 ce 02 00 00 jne 0x2f4
>> 26: e9 fd 00 00 00 jmpq 0x128
>> 2b:* 0f b6 47 2a movzbl 0x2a(%rdi),%eax <-- trapping
>> instruction
>
> Above instruction is 'sel->prefixlen_d' from
> the addr4_match call in __xfrm4_selector_match. Looks like
> we dereference sel (%rdi) with bad value of 00000000000600a6.
> xfrm_sk_policy_lookup() provides &pol->selector to
> xfrm_selector_match, so pol has a bad value. I don't remember
> for such problem, not sure if the 3-hour period is some timer
> in xfrm.
>
In fact it could be timer related:
1st. try
[13061.933733] IP: [<ffffffff8154f5a3>] xfrm_selector_match+0x25/0x2f6
[13061.934440] RIP: 0010:[<ffffffff8154f5a3>] [<ffffffff8154f5a3>]
xfrm_selector_match+0x25/0x2f6
[13061.936477] RIP [<ffffffff8154f5a3>] xfrm_selector_match+0x25/0x2f6
2nd. try
[13230.422541] IP: [<ffffffff814ff2fc>] xfrm_selector_match+0x25/0x2f6
[13230.423440] RIP: 0010:[<ffffffff814ff2fc>] [<ffffffff814ff2fc>]
xfrm_selector_match+0x25/0x2f6
[13230.431740] RIP [<ffffffff814ff2fc>] xfrm_selector_match+0x25/0x2f6
>> Could someone shed some light on the decoded output and point me somewhere so i
>> can debug this further?
>
> If noone else has idea what can be wrong, can you try
> some kernels between 3.10.40 and 3.12.33 or even some lastest
> kernel?
>
I tried 3.17.4 which seems not have this issue any more, but has another
regression in ocfs2 which is why we cannot use it.
3.10.61 looks fine so far, but i cannot tell for sure, uptime is 1:23 right now,
i'll keep you updated.
--
Mit freundlichen Grüßen,
Florian Wiessner
Smart Weblications GmbH
Martinsberger Str. 1
D-95119 Naila
fon.: +49 9282 9638 200
fax.: +49 9282 9638 205
24/7: +49 900 144 000 00 - 0,99 EUR/Min*
http://www.smart-weblications.de
--
Sitz der Gesellschaft: Naila
Geschäftsführer: Florian Wiessner
HRB-Nr.: HRB 3840 Amtsgericht Hof
*aus dem dt. Festnetz, ggf. abweichende Preise aus dem Mobilfunknetz
^ permalink raw reply
* RE: [PATCH v3] can: Convert to runtime_pm
From: Appana Durga Kedareswara Rao @ 2014-11-28 1:49 UTC (permalink / raw)
To: Soren Brinkmann
Cc: wg@grandegger.com, mkl@pengutronix.de, Michal Simek,
grant.likely@linaro.org, robh+dt@kernel.org,
devicetree@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-can@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20141127182337.GF17884@xsjandreislx>
Hi Soren,
-----Original Message-----
From: Sören Brinkmann [mailto:soren.brinkmann@xilinx.com]
Sent: Thursday, November 27, 2014 11:54 PM
To: Appana Durga Kedareswara Rao
Cc: wg@grandegger.com; mkl@pengutronix.de; Michal Simek; grant.likely@linaro.org; robh+dt@kernel.org; devicetree@vger.kernel.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; linux-can@vger.kernel.org; Appana Durga Kedareswara Rao; linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3] can: Convert to runtime_pm
Hi Kedar,
On Thu, 2014-11-27 at 06:38PM +0530, Kedareswara rao Appana wrote:
> Instead of enabling/disabling clocks at several locations in the
> driver, use the runtime_pm framework. This consolidates the actions
> for runtime PM in the appropriate callbacks and makes the driver more
> readable and mantainable.
>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> ---
> Changes for v3:
> - Converted the driver to use runtime_pm.
> Changes for v2:
> - Removed the struct platform_device* from suspend/resume
> as suggest by Lothar.
>
> drivers/net/can/xilinx_can.c | 119
> +++++++++++++++++++++++++----------------
> 1 files changed, 72 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/can/xilinx_can.c
> b/drivers/net/can/xilinx_can.c index 8a998e3..1be28ed 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -32,6 +32,7 @@
> #include <linux/can/dev.h>
> #include <linux/can/error.h>
> #include <linux/can/led.h>
> +#include <linux/pm_runtime.h>
>
> #define DRIVER_NAME "xilinx_can"
>
> @@ -138,7 +139,7 @@ struct xcan_priv {
> u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
> void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
> u32 val);
> - struct net_device *dev;
> + struct device *dev;
> void __iomem *reg_base;
> unsigned long irq_flags;
> struct clk *bus_clk;
> @@ -842,6 +843,13 @@ static int xcan_open(struct net_device *ndev)
> struct xcan_priv *priv = netdev_priv(ndev);
> int ret;
>
> + ret = pm_runtime_get_sync(priv->dev);
> + if (ret < 0) {
> + netdev_err(ndev, "%s: runtime CAN resume failed(%d)\n\r",
There might be other issues than the resume that make this fail. It should probably just say 'pm_runtime_get failed'.
The CAN in the string should not be needed, the netdev_err macro makes sure the device name is printed.
Can we have a space between 'failed' and the error code?
There should not be a '\r'
K sure will modify the error message as you explained above.
> + __func__, ret);
> + return ret;
> + }
> +
> ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
> ndev->name, ndev);
> if (ret < 0) {
> @@ -849,29 +857,17 @@ static int xcan_open(struct net_device *ndev)
> goto err;
> }
>
> - ret = clk_prepare_enable(priv->can_clk);
> - if (ret) {
> - netdev_err(ndev, "unable to enable device clock\n");
> - goto err_irq;
> - }
> -
> - ret = clk_prepare_enable(priv->bus_clk);
> - if (ret) {
> - netdev_err(ndev, "unable to enable bus clock\n");
> - goto err_can_clk;
> - }
> -
> /* Set chip into reset mode */
> ret = set_reset_mode(ndev);
> if (ret < 0) {
> netdev_err(ndev, "mode resetting failed!\n");
> - goto err_bus_clk;
> + goto err_irq;
> }
>
> /* Common open */
> ret = open_candev(ndev);
> if (ret)
> - goto err_bus_clk;
> + goto err_irq;
>
> ret = xcan_chip_start(ndev);
> if (ret < 0) {
> @@ -887,13 +883,11 @@ static int xcan_open(struct net_device *ndev)
>
> err_candev:
> close_candev(ndev);
> -err_bus_clk:
> - clk_disable_unprepare(priv->bus_clk);
> -err_can_clk:
> - clk_disable_unprepare(priv->can_clk);
> err_irq:
> free_irq(ndev->irq, ndev);
> err:
> + pm_runtime_put(priv->dev);
> +
> return ret;
> }
>
> @@ -910,12 +904,11 @@ static int xcan_close(struct net_device *ndev)
> netif_stop_queue(ndev);
> napi_disable(&priv->napi);
> xcan_chip_stop(ndev);
> - clk_disable_unprepare(priv->bus_clk);
> - clk_disable_unprepare(priv->can_clk);
> free_irq(ndev->irq, ndev);
> close_candev(ndev);
>
> can_led_event(ndev, CAN_LED_EVENT_STOP);
> + pm_runtime_put(priv->dev);
>
> return 0;
> }
> @@ -934,27 +927,21 @@ static int xcan_get_berr_counter(const struct net_device *ndev,
> struct xcan_priv *priv = netdev_priv(ndev);
> int ret;
>
> - ret = clk_prepare_enable(priv->can_clk);
> - if (ret)
> - goto err;
> + ret = pm_runtime_get_sync(priv->dev);
> + if (ret < 0) {
> + netdev_err(ndev, "%s: runtime resume failed(%d)\n\r",
> + __func__, ret);
As above.
K will modify this too.
Regards,
Kedar.
Sören
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ 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