From: Bin Meng <bmeng.cn@gmail.com> To: "Philippe Mathieu-Daudé" <philmd@redhat.com> Cc: "Peter Maydell" <peter.maydell@linaro.org>, "Bin Meng" <bin.meng@windriver.com>, "Li Zhijian" <lizhijian@cn.fujitsu.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Andrew Jeffery" <andrew@aj.id.au>, "Jason Wang" <jasowang@redhat.com>, "Alistair Francis" <alistair@alistair23.me>, "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>, "Paul Durrant" <paul@xen.org>, "Zhang Chen" <chen.zhang@intel.com>, "Stefano Stabellini" <sstabellini@kernel.org>, "Peter Chubb" <peter.chubb@nicta.com.au>, "Joel Stanley" <joel@jms.id.au>, "Beniamino Galvani" <b.galvani@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, "Anthony Perard" <anthony.perard@citrix.com>, xen-devel@lists.xenproject.org, "David Gibson" <david@gibson.dropbear.id.au>, qemu-ppc@nongnu.org, qemu-arm <qemu-arm@nongnu.org>, "Cédric Le Goater" <clg@kaod.org> Subject: Re: [PATCH 3/3] net: checksum: Introduce fine control over checksum type Date: Sun, 6 Dec 2020 20:44:40 +0800 [thread overview] Message-ID: <CAEUhbmUBeUzjPG=2-WF=UnpMnVkH3qT0FkXpgBhP==yt53UfBg@mail.gmail.com> (raw) In-Reply-To: <9f8cdb69-7b74-4034-223f-bfa62cb4e440@redhat.com> Hi Philippe, On Sun, Dec 6, 2020 at 7:50 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Hi Ben, > > On 12/6/20 3:14 AM, Bin Meng wrote: > > From: Bin Meng <bin.meng@windriver.com> > > > > At present net_checksum_calculate() blindly calculates all types of > > checksums (IP, TCP, UDP). Some NICs may have a per type setting in > > their BDs to control what checksum should be offloaded. To support > > such hardware behavior, introduce a 'csum_flag' parameter to the > > net_checksum_calculate() API to allow fine control over what type > > checksum is calculated. > > > > Existing users of this API are updated accordingly. > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > > > --- > > > > hw/net/allwinner-sun8i-emac.c | 2 +- > > hw/net/cadence_gem.c | 2 +- > > hw/net/fsl_etsec/rings.c | 8 +++----- > > hw/net/ftgmac100.c | 10 +++++++++- > > hw/net/imx_fec.c | 15 +++------------ > > hw/net/virtio-net.c | 2 +- > > hw/net/xen_nic.c | 2 +- > > include/net/checksum.h | 7 ++++++- > > When sending a such API refactor, patch is easier to > review if you setup the scripts/git.orderfile config. Sure. I thought I have done this before but apparently not on the machine this series was genearated :) > > > net/checksum.c | 18 ++++++++++++++---- > > net/filter-rewriter.c | 4 ++-- > > 10 files changed, 41 insertions(+), 29 deletions(-) > ... > > diff --git a/include/net/checksum.h b/include/net/checksum.h > > index 05a0d27..7dec37e 100644 > > --- a/include/net/checksum.h > > +++ b/include/net/checksum.h > > @@ -21,11 +21,16 @@ > > #include "qemu/bswap.h" > > struct iovec; > > > > +#define CSUM_IP 0x01 > > IMO this is IP_HEADER, Yes, but I believe no one will misread it, no? > > > +#define CSUM_TCP 0x02 > > +#define CSUM_UDP 0x04 > > and these IP_PAYLOAD, regardless the payload protocol. We have to distinguish TCP and UDP. > > > +#define CSUM_ALL (CSUM_IP | CSUM_TCP | CSUM_UDP) > > Maybe CSUM_HEADER / CSUM_PAYLOAD / CSUM_FULL (aka RAW?). > > > + > > uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq); > > uint16_t net_checksum_finish(uint32_t sum); > > uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto, > > uint8_t *addrs, uint8_t *buf); > > -void net_checksum_calculate(uint8_t *data, int length); > > +void net_checksum_calculate(uint8_t *data, int length, int csum_flag); > > > > static inline uint32_t > > net_checksum_add(int len, uint8_t *buf) > > diff --git a/net/checksum.c b/net/checksum.c > > index dabd290..70f4eae 100644 > > --- a/net/checksum.c > > +++ b/net/checksum.c > > @@ -57,7 +57,7 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto, > > return net_checksum_finish(sum); > > } > > > > -void net_checksum_calculate(uint8_t *data, int length) > > +void net_checksum_calculate(uint8_t *data, int length, int csum_flag) > > { > > int mac_hdr_len, ip_len; > > struct ip_header *ip; > > @@ -108,9 +108,11 @@ void net_checksum_calculate(uint8_t *data, int length) > > } > > > > /* Calculate IP checksum */ > > - stw_he_p(&ip->ip_sum, 0); > > - csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip)); > > - stw_be_p(&ip->ip_sum, csum); > > + if (csum_flag & CSUM_IP) { > > + stw_he_p(&ip->ip_sum, 0); > > + csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip)); > > + stw_be_p(&ip->ip_sum, csum); > > + } > > > > if (IP4_IS_FRAGMENT(ip)) { > > return; /* a fragmented IP packet */ > > @@ -128,6 +130,10 @@ void net_checksum_calculate(uint8_t *data, int length) > > switch (ip->ip_p) { > > case IP_PROTO_TCP: > > { > > + if (!(csum_flag & CSUM_TCP)) { > > + return; > > + } > > + > > tcp_header *tcp = (tcp_header *)(ip + 1); > > > > if (ip_len < sizeof(tcp_header)) { > > @@ -148,6 +154,10 @@ void net_checksum_calculate(uint8_t *data, int length) > > } > > case IP_PROTO_UDP: > > { > > + if (!(csum_flag & CSUM_UDP)) { > > + return; > > + } > > + > > udp_header *udp = (udp_header *)(ip + 1); > > > > if (ip_len < sizeof(udp_header)) { > ... Regards, Bin
WARNING: multiple messages have this Message-ID (diff)
From: Bin Meng <bmeng.cn@gmail.com> To: "Philippe Mathieu-Daudé" <philmd@redhat.com> Cc: "qemu-devel@nongnu.org Developers" <qemu-devel@nongnu.org>, "Peter Maydell" <peter.maydell@linaro.org>, "Alistair Francis" <alistair@alistair23.me>, "Paul Durrant" <paul@xen.org>, "Li Zhijian" <lizhijian@cn.fujitsu.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Andrew Jeffery" <andrew@aj.id.au>, "Jason Wang" <jasowang@redhat.com>, "Bin Meng" <bin.meng@windriver.com>, "Joel Stanley" <joel@jms.id.au>, "Beniamino Galvani" <b.galvani@gmail.com>, "Zhang Chen" <chen.zhang@intel.com>, "Stefano Stabellini" <sstabellini@kernel.org>, "Peter Chubb" <peter.chubb@nicta.com.au>, "Cédric Le Goater" <clg@kaod.org>, qemu-arm <qemu-arm@nongnu.org>, xen-devel@lists.xenproject.org, "Anthony Perard" <anthony.perard@citrix.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, qemu-ppc@nongnu.org, "David Gibson" <david@gibson.dropbear.id.au> Subject: Re: [PATCH 3/3] net: checksum: Introduce fine control over checksum type Date: Sun, 6 Dec 2020 20:44:40 +0800 [thread overview] Message-ID: <CAEUhbmUBeUzjPG=2-WF=UnpMnVkH3qT0FkXpgBhP==yt53UfBg@mail.gmail.com> (raw) In-Reply-To: <9f8cdb69-7b74-4034-223f-bfa62cb4e440@redhat.com> Hi Philippe, On Sun, Dec 6, 2020 at 7:50 PM Philippe Mathieu-Daudé <philmd@redhat.com> wrote: > > Hi Ben, > > On 12/6/20 3:14 AM, Bin Meng wrote: > > From: Bin Meng <bin.meng@windriver.com> > > > > At present net_checksum_calculate() blindly calculates all types of > > checksums (IP, TCP, UDP). Some NICs may have a per type setting in > > their BDs to control what checksum should be offloaded. To support > > such hardware behavior, introduce a 'csum_flag' parameter to the > > net_checksum_calculate() API to allow fine control over what type > > checksum is calculated. > > > > Existing users of this API are updated accordingly. > > > > Signed-off-by: Bin Meng <bin.meng@windriver.com> > > > > --- > > > > hw/net/allwinner-sun8i-emac.c | 2 +- > > hw/net/cadence_gem.c | 2 +- > > hw/net/fsl_etsec/rings.c | 8 +++----- > > hw/net/ftgmac100.c | 10 +++++++++- > > hw/net/imx_fec.c | 15 +++------------ > > hw/net/virtio-net.c | 2 +- > > hw/net/xen_nic.c | 2 +- > > include/net/checksum.h | 7 ++++++- > > When sending a such API refactor, patch is easier to > review if you setup the scripts/git.orderfile config. Sure. I thought I have done this before but apparently not on the machine this series was genearated :) > > > net/checksum.c | 18 ++++++++++++++---- > > net/filter-rewriter.c | 4 ++-- > > 10 files changed, 41 insertions(+), 29 deletions(-) > ... > > diff --git a/include/net/checksum.h b/include/net/checksum.h > > index 05a0d27..7dec37e 100644 > > --- a/include/net/checksum.h > > +++ b/include/net/checksum.h > > @@ -21,11 +21,16 @@ > > #include "qemu/bswap.h" > > struct iovec; > > > > +#define CSUM_IP 0x01 > > IMO this is IP_HEADER, Yes, but I believe no one will misread it, no? > > > +#define CSUM_TCP 0x02 > > +#define CSUM_UDP 0x04 > > and these IP_PAYLOAD, regardless the payload protocol. We have to distinguish TCP and UDP. > > > +#define CSUM_ALL (CSUM_IP | CSUM_TCP | CSUM_UDP) > > Maybe CSUM_HEADER / CSUM_PAYLOAD / CSUM_FULL (aka RAW?). > > > + > > uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq); > > uint16_t net_checksum_finish(uint32_t sum); > > uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto, > > uint8_t *addrs, uint8_t *buf); > > -void net_checksum_calculate(uint8_t *data, int length); > > +void net_checksum_calculate(uint8_t *data, int length, int csum_flag); > > > > static inline uint32_t > > net_checksum_add(int len, uint8_t *buf) > > diff --git a/net/checksum.c b/net/checksum.c > > index dabd290..70f4eae 100644 > > --- a/net/checksum.c > > +++ b/net/checksum.c > > @@ -57,7 +57,7 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t proto, > > return net_checksum_finish(sum); > > } > > > > -void net_checksum_calculate(uint8_t *data, int length) > > +void net_checksum_calculate(uint8_t *data, int length, int csum_flag) > > { > > int mac_hdr_len, ip_len; > > struct ip_header *ip; > > @@ -108,9 +108,11 @@ void net_checksum_calculate(uint8_t *data, int length) > > } > > > > /* Calculate IP checksum */ > > - stw_he_p(&ip->ip_sum, 0); > > - csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip)); > > - stw_be_p(&ip->ip_sum, csum); > > + if (csum_flag & CSUM_IP) { > > + stw_he_p(&ip->ip_sum, 0); > > + csum = net_raw_checksum((uint8_t *)ip, IP_HDR_GET_LEN(ip)); > > + stw_be_p(&ip->ip_sum, csum); > > + } > > > > if (IP4_IS_FRAGMENT(ip)) { > > return; /* a fragmented IP packet */ > > @@ -128,6 +130,10 @@ void net_checksum_calculate(uint8_t *data, int length) > > switch (ip->ip_p) { > > case IP_PROTO_TCP: > > { > > + if (!(csum_flag & CSUM_TCP)) { > > + return; > > + } > > + > > tcp_header *tcp = (tcp_header *)(ip + 1); > > > > if (ip_len < sizeof(tcp_header)) { > > @@ -148,6 +154,10 @@ void net_checksum_calculate(uint8_t *data, int length) > > } > > case IP_PROTO_UDP: > > { > > + if (!(csum_flag & CSUM_UDP)) { > > + return; > > + } > > + > > udp_header *udp = (udp_header *)(ip + 1); > > > > if (ip_len < sizeof(udp_header)) { > ... Regards, Bin
next prev parent reply other threads:[~2020-12-06 12:49 UTC|newest] Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-06 2:14 [PATCH 1/3] net: checksum: Skip fragmented IP packets Bin Meng 2020-12-06 2:14 ` [PATCH 2/3] net: checksum: Add IP header checksum calculation Bin Meng 2020-12-06 2:14 ` [PATCH 3/3] net: checksum: Introduce fine control over checksum type Bin Meng 2020-12-06 2:14 ` Bin Meng 2020-12-06 11:50 ` Philippe Mathieu-Daudé 2020-12-06 11:50 ` Philippe Mathieu-Daudé 2020-12-06 12:44 ` Bin Meng [this message] 2020-12-06 12:44 ` Bin Meng 2020-12-09 8:33 ` Cédric Le Goater 2020-12-09 8:33 ` Cédric Le Goater
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='CAEUhbmUBeUzjPG=2-WF=UnpMnVkH3qT0FkXpgBhP==yt53UfBg@mail.gmail.com' \ --to=bmeng.cn@gmail.com \ --cc=alistair@alistair23.me \ --cc=andrew@aj.id.au \ --cc=anthony.perard@citrix.com \ --cc=b.galvani@gmail.com \ --cc=bin.meng@windriver.com \ --cc=chen.zhang@intel.com \ --cc=clg@kaod.org \ --cc=david@gibson.dropbear.id.au \ --cc=edgar.iglesias@gmail.com \ --cc=jasowang@redhat.com \ --cc=joel@jms.id.au \ --cc=lizhijian@cn.fujitsu.com \ --cc=mst@redhat.com \ --cc=paul@xen.org \ --cc=peter.chubb@nicta.com.au \ --cc=peter.maydell@linaro.org \ --cc=philmd@redhat.com \ --cc=qemu-arm@nongnu.org \ --cc=qemu-devel@nongnu.org \ --cc=qemu-ppc@nongnu.org \ --cc=sstabellini@kernel.org \ --cc=xen-devel@lists.xenproject.org \ /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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.