qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Anton Ivanov (antivano)" <antivano@cisco.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Qemu-devel@nongnu.org" <Qemu-devel@nongnu.org>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: Re: [Qemu-devel] Contribution - L2TPv3 transport
Date: Tue, 4 Mar 2014 17:28:34 +0000	[thread overview]
Message-ID: <53160D40.8010800@cisco.com> (raw)
In-Reply-To: <5316057E.8000304@redhat.com>

On 04/03/14 16:55, Paolo Bonzini wrote:
> Il 04/03/2014 17:48, Anton Ivanov (antivano) ha scritto:
>>>> >> +static ssize_t net_l2tpv3_receive_dgram_iov(NetClientState *nc, 
>>>> const struct iovec *iov, int iovcnt)
>>> > Long line; you can split after , to fit within 80 columns.
>> OK
>>> >
>>>> >> +{
>>>> >> +    NetL2TPV3State *s = DO_UPCAST(NetL2TPV3State, nc, nc);
>>>> >> +
>>>> >> +    struct msghdr message;
>>>> >> +    int ret;
>>>> >> +
>>>> >> +    if (iovcnt > MAX_L2TPV3_IOVCNT - 1) {
>>>> >> +    fprintf(stderr, "iovec too long %d > %d, change 
>>>> l2tpv3.h\n", iovcnt, MAX_L2TPV3_IOVCNT);
>>>> >> +    return -1;
>>> > Is printing to stderr always the right thing to do?  It seems to 
>>> me that
>>> > you should look into using QError.
>> Thanks, will look into it.
>>
>
> Actually no, this does not need to use QError.  You just need 
> "error_report", which is the same as fprintf(stderr) but will add nice 
> timestamps in front of the error message if enabled.

Everything updated to that, Eric's comments (except QError and bus 
alignment) taken in to account too.

I have done a compromise on the buffer at the moment and set it to 2048. 
I need to understand fully how this gets paged in and how to make that 
process optimal, then we will adjust it. The idea is to expand on this 
in the future to allow jumbo frames (up to 8k).

I will rebuild it, retest it to make sure it works and provide an 
updated version tomorrow.

One question - what git client are you using to do git send-mail. While 
debian does have a reference to git send-mail in the main git manpage, 
it does not seem to have it as a part of git.

By the way - apologies for the code quality, as you have probably 
guessed, I am not a "proper" developer - I am a network 
engineer/sysadmin by trade.

A.

>
> Paolo

  reply	other threads:[~2014-03-04 17:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  8:28 [Qemu-devel] Contribution - L2TPv3 transport Anton Ivanov (antivano)
2014-02-28 10:02 ` Paolo Bonzini
2014-02-28 11:17   ` Anton Ivanov (antivano)
2014-02-28 11:36     ` Paolo Bonzini
2014-02-28 12:59       ` Anton Ivanov (antivano)
2014-02-28 13:55   ` Anton Ivanov (antivano)
2014-03-04 15:19   ` Anton Ivanov (antivano)
2014-03-04 15:22     ` Anton Ivanov (antivano)
2014-03-04 15:53       ` Eric Blake
2014-03-04 16:05         ` Anton Ivanov (antivano)
2014-03-05  8:49         ` Anton Ivanov (antivano)
2014-03-05 11:38           ` Peter Maydell
2014-03-04 15:41     ` Eric Blake
2014-03-04 15:58       ` Anton Ivanov (antivano)
2014-03-04 16:04         ` Paolo Bonzini
2014-03-04 16:33     ` Eric Blake
2014-03-04 16:48       ` Anton Ivanov (antivano)
2014-03-04 16:55         ` Paolo Bonzini
2014-03-04 17:28           ` Anton Ivanov (antivano) [this message]
2014-03-04 17:30             ` Paolo Bonzini
2014-02-28 13:40 ` Eric Blake
2014-02-28 13:52   ` Anton Ivanov (antivano)
2014-02-28 13:57     ` Eric Blake
2014-02-28 14:03       ` Anton Ivanov (antivano)
2014-02-28 14:00   ` Paolo Bonzini
2014-02-28 15:06     ` Eric Blake
2014-02-28 15:20       ` Paolo Bonzini
2014-03-03 13:27 ` Stefan Hajnoczi
2014-03-03 14:01   ` Anton Ivanov (antivano)
2014-03-04  9:36     ` Stefan Hajnoczi
2014-03-04  9:47       ` Anton Ivanov (antivano)
2014-03-05  8:59         ` Stefan Hajnoczi
2014-03-05  9:13           ` Vincenzo Maffione
2014-03-03 14:53 ` Stefan Hajnoczi
2014-03-04 11:32   ` Anton Ivanov (antivano)
2014-03-05  9:07     ` Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53160D40.8010800@cisco.com \
    --to=antivano@cisco.com \
    --cc=Qemu-devel@nongnu.org \
    --cc=pbonzini@redhat.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).