Patchwork [v11,2/5] iovec checksum calculation fuction

login
register
mail settings
Submitter Dmitry Fleytman
Date Feb. 25, 2013, 8:11 p.m.
Message ID <1361823063-26713-3-git-send-email-dmitry@daynix.com>
Download mbox | patch
Permalink /patch/223028/
State New
Headers show

Comments

Dmitry Fleytman - Feb. 25, 2013, 8:11 p.m.
Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
Signed-off-by: Yan Vugenfirer <yan@daynix.com>
---
 include/net/checksum.h |  8 ++++++++
 net/checksum.c         | 28 ++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+)
Andreas Färber - Feb. 25, 2013, 8:37 p.m.
Am 25.02.2013 21:11, schrieb Dmitry Fleytman:
> Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> Signed-off-by: Yan Vugenfirer <yan@daynix.com>
> ---
>  include/net/checksum.h |  8 ++++++++
>  net/checksum.c         | 28 ++++++++++++++++++++++++++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/include/net/checksum.h b/include/net/checksum.h
> index 3e7b93d..b1cf18a 100644
> --- a/include/net/checksum.h
> +++ b/include/net/checksum.h
> @@ -19,6 +19,7 @@
>  #define QEMU_NET_CHECKSUM_H
>  
>  #include <stdint.h>
> +#include "qemu-common.h"

Eduardo has worked hard to resolve circular qemu-common.h dependencies!
Are you sure you are not reintroducing one here? What do you actually
need out of it? You already have stdint.h for uint32_t, and struct iovec
is used as pointer so you shouldn't need its internals from
qemu-common.h here and can include it from checksum.c instead.

>  
>  uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq);
>  uint16_t net_checksum_finish(uint32_t sum);
> @@ -38,4 +39,11 @@ net_raw_checksum(uint8_t *data, int length)
>      return net_checksum_finish(net_checksum_add(length, data));
>  }
>  
> +/**
 * net_checksum_add_iov:
 * @iov: ...
 * @iov_cnt: ...
 * @iov_off: ...
 * @size: ...
 *
> + * Checksum calculation for scatter-gather vector
> + */
> +uint32_t net_checksum_add_iov(const struct iovec *iov,
> +                              const unsigned int iov_cnt,
> +                              uint32_t iov_off, uint32_t size);
> +
>  #endif /* QEMU_NET_CHECKSUM_H */
[snip]

The subject is also improvable: "net: ", an appropriate verb and a typo.

Regards,
Andreas
Eduardo Habkost - Feb. 26, 2013, 1:38 p.m.
On Mon, Feb 25, 2013 at 09:37:52PM +0100, Andreas Färber wrote:
> Am 25.02.2013 21:11, schrieb Dmitry Fleytman:
[...]
> > diff --git a/include/net/checksum.h b/include/net/checksum.h
> > index 3e7b93d..b1cf18a 100644
> > --- a/include/net/checksum.h
> > +++ b/include/net/checksum.h
> > @@ -19,6 +19,7 @@
> >  #define QEMU_NET_CHECKSUM_H
> >  
> >  #include <stdint.h>
> > +#include "qemu-common.h"
> 
> Eduardo has worked hard to resolve circular qemu-common.h dependencies!
> Are you sure you are not reintroducing one here?

Even if there's no circular dependency yet, this makes it very easy to
introduce circular dependencies silently if one day a header included by
qemu-common.h ends up including checksum.h. That's why qemu-common.h
shouldn't be included by any header file.


> What do you actually
> need out of it? You already have stdint.h for uint32_t, and struct iovec
> is used as pointer so you shouldn't need its internals from
> qemu-common.h here and can include it from checksum.c instead.
> 
[...]
Dmitry Fleytman - March 2, 2013, 12:28 p.m.
Eduardo, Andreas,

Thanks for pointing out, we didn't know about this limitation of
qemu-common.h usage.
Include directive will be moved to corresponding .c file.

Dmitry.


On Tue, Feb 26, 2013 at 3:38 PM, Eduardo Habkost <ehabkost@redhat.com>wrote:

> On Mon, Feb 25, 2013 at 09:37:52PM +0100, Andreas Färber wrote:
> > Am 25.02.2013 21:11, schrieb Dmitry Fleytman:
> [...]
> > > diff --git a/include/net/checksum.h b/include/net/checksum.h
> > > index 3e7b93d..b1cf18a 100644
> > > --- a/include/net/checksum.h
> > > +++ b/include/net/checksum.h
> > > @@ -19,6 +19,7 @@
> > >  #define QEMU_NET_CHECKSUM_H
> > >
> > >  #include <stdint.h>
> > > +#include "qemu-common.h"
> >
> > Eduardo has worked hard to resolve circular qemu-common.h dependencies!
> > Are you sure you are not reintroducing one here?
>
> Even if there's no circular dependency yet, this makes it very easy to
> introduce circular dependencies silently if one day a header included by
> qemu-common.h ends up including checksum.h. That's why qemu-common.h
> shouldn't be included by any header file.
>
>
> > What do you actually
> > need out of it? You already have stdint.h for uint32_t, and struct iovec
> > is used as pointer so you shouldn't need its internals from
> > qemu-common.h here and can include it from checksum.c instead.
> >
> [...]
>
> --
> Eduardo
>
Dmitry Fleytman - March 2, 2013, 12:30 p.m.
Thanks Andreas,
fixed.

On Mon, Feb 25, 2013 at 10:37 PM, Andreas Färber <afaerber@suse.de> wrote:

> Am 25.02.2013 21:11, schrieb Dmitry Fleytman:
> > Signed-off-by: Dmitry Fleytman <dmitry@daynix.com>
> > Signed-off-by: Yan Vugenfirer <yan@daynix.com>
> > ---
> >  include/net/checksum.h |  8 ++++++++
> >  net/checksum.c         | 28 ++++++++++++++++++++++++++++
> >  2 files changed, 36 insertions(+)
> >
> > diff --git a/include/net/checksum.h b/include/net/checksum.h
> > index 3e7b93d..b1cf18a 100644
> > --- a/include/net/checksum.h
> > +++ b/include/net/checksum.h
> > @@ -19,6 +19,7 @@
> >  #define QEMU_NET_CHECKSUM_H
> >
> >  #include <stdint.h>
> > +#include "qemu-common.h"
>
> Eduardo has worked hard to resolve circular qemu-common.h dependencies!
> Are you sure you are not reintroducing one here? What do you actually
> need out of it? You already have stdint.h for uint32_t, and struct iovec
> is used as pointer so you shouldn't need its internals from
> qemu-common.h here and can include it from checksum.c instead.
>
> >
> >  uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq);
> >  uint16_t net_checksum_finish(uint32_t sum);
> > @@ -38,4 +39,11 @@ net_raw_checksum(uint8_t *data, int length)
> >      return net_checksum_finish(net_checksum_add(length, data));
> >  }
> >
> > +/**
>  * net_checksum_add_iov:
>  * @iov: ...
>  * @iov_cnt: ...
>  * @iov_off: ...
>  * @size: ...
>  *
> > + * Checksum calculation for scatter-gather vector
> > + */
> > +uint32_t net_checksum_add_iov(const struct iovec *iov,
> > +                              const unsigned int iov_cnt,
> > +                              uint32_t iov_off, uint32_t size);
> > +
> >  #endif /* QEMU_NET_CHECKSUM_H */
> [snip]
>
> The subject is also improvable: "net: ", an appropriate verb and a typo.
>
> Regards,
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
>

Patch

diff --git a/include/net/checksum.h b/include/net/checksum.h
index 3e7b93d..b1cf18a 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -19,6 +19,7 @@ 
 #define QEMU_NET_CHECKSUM_H
 
 #include <stdint.h>
+#include "qemu-common.h"
 
 uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq);
 uint16_t net_checksum_finish(uint32_t sum);
@@ -38,4 +39,11 @@  net_raw_checksum(uint8_t *data, int length)
     return net_checksum_finish(net_checksum_add(length, data));
 }
 
+/**
+ * Checksum calculation for scatter-gather vector
+ */
+uint32_t net_checksum_add_iov(const struct iovec *iov,
+                              const unsigned int iov_cnt,
+                              uint32_t iov_off, uint32_t size);
+
 #endif /* QEMU_NET_CHECKSUM_H */
diff --git a/net/checksum.c b/net/checksum.c
index 4fa5563..9c813ff 100644
--- a/net/checksum.c
+++ b/net/checksum.c
@@ -84,3 +84,31 @@  void net_checksum_calculate(uint8_t *data, int length)
     data[14+hlen+csum_offset]   = csum >> 8;
     data[14+hlen+csum_offset+1] = csum & 0xff;
 }
+
+uint32_t
+net_checksum_add_iov(const struct iovec *iov, const unsigned int iov_cnt,
+                     uint32_t iov_off, uint32_t size)
+{
+    size_t iovec_off, buf_off;
+    unsigned int i;
+    uint32_t res = 0;
+    uint32_t seq = 0;
+
+    iovec_off = 0;
+    buf_off = 0;
+    for (i = 0; i < iov_cnt && size; i++) {
+        if (iov_off < (iovec_off + iov[i].iov_len)) {
+            size_t len = MIN((iovec_off + iov[i].iov_len) - iov_off , size);
+            void *chunk_buf = iov[i].iov_base + (iov_off - iovec_off);
+
+            res += net_checksum_add_cont(len, chunk_buf, seq);
+            seq += len;
+
+            buf_off += len;
+            iov_off += len;
+            size -= len;
+        }
+        iovec_off += iov[i].iov_len;
+    }
+    return res;
+}