| Message ID | 20180529062838.20556-7-clg@kaod.org |
|---|---|
| State | New |
| Headers | show |
| Series | ftgmac100 and NC-SI enhancements for the Aspeed SoC | expand |
On 05/29/2018 03:28 AM, Cédric Le Goater wrote: > The checksum field of a NC-SI packet contains a value that may be > included in each command and response. The verification is optional > but the Linux driver does so when a non-zero value is provided. Let's > extend the model to compute the checksum value and exercise a little > more the Linux driver. Due to - "If the originator of an NC-SI Control packet is not generating a checksum, the originator shall use a value of all zeros for the header checksum field." - "All receivers of NC-SI Control packets shall accept packets with all zeros as the checksum value (provided that other fields and the CRC are correct)." - "The receiver of an NC-SI Control packet may reject (silently discard) a packet that has an incorrect non-zero checksum." I was tempted to suggest you to check the input checksum if any, but "A controller that generates checksums is not required to verify checksums that it receives." so we don't care. > > See section "8.2.2.3 - 2's Complement Checksum Compensation" in the > Network Controller Sideband Interface (NC-SI) Specification for more > details. > > Signed-off-by: Cédric Le Goater <clg@kaod.org> > --- > slirp/ncsi.c | 29 +++++++++++++++++++++++++---- > 1 file changed, 25 insertions(+), 4 deletions(-) > > diff --git a/slirp/ncsi.c b/slirp/ncsi.c > index f00253641ea4..fbfb79713f23 100644 > --- a/slirp/ncsi.c > +++ b/slirp/ncsi.c > @@ -8,9 +8,23 @@ > */ > #include "qemu/osdep.h" > #include "slirp.h" > +#include "net/checksum.h" > > #include "ncsi-pkt.h" > > +static uint32_t ncsi_calculate_checksum(unsigned char *data, int len) "NC-SI packet payload interpreted as a series of 16-bit unsigned integer values." Can you use the "uint32_t ncsi_calculate_checksum(uint16_t *data, size_t length)" prototype instead if you respin? Regardless: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > +{ > + uint32_t checksum = 0; > + int i; > + > + for (i = 0; i < len; i += 2) { > + checksum += (((uint32_t)data[i] << 8) | data[i + 1]); > + } > + > + checksum = (~checksum + 1); > + return checksum; > +} > + > /* Get Capabilities */ > static int ncsi_rsp_handler_gc(struct ncsi_rsp_pkt_hdr *rnh) > { > @@ -108,6 +122,9 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) > (ncsi_reply + ETH_HLEN); > const struct ncsi_rsp_handler *handler = NULL; > int i; > + int ncsi_rsp_len = sizeof(*nh); > + uint32_t checksum; > + uint32_t *pchecksum; > > memset(ncsi_reply, 0, sizeof(ncsi_reply)); > > @@ -137,15 +154,19 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) > /* TODO: handle errors */ > handler->handler(rnh); > } > + ncsi_rsp_len += handler->payload; > } else { > rnh->common.length = 0; > rnh->code = htons(NCSI_PKT_RSP_C_UNAVAILABLE); > rnh->reason = htons(NCSI_PKT_RSP_R_UNKNOWN); > } > > - /* TODO: add a checksum at the end of the frame but the specs > - * allows it to be zero */ > + /* add a checksum at the end of the frame (the specs allows it to > + * be zero */ > + checksum = ncsi_calculate_checksum((unsigned char *) rnh, ncsi_rsp_len); > + pchecksum = (uint32_t *)((void *) rnh + ncsi_rsp_len); > + *pchecksum = htonl(checksum); > + ncsi_rsp_len += 4; > > - slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + sizeof(*nh) + > - (handler ? handler->payload : 0) + 4); > + slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + ncsi_rsp_len); > } >
diff --git a/slirp/ncsi.c b/slirp/ncsi.c index f00253641ea4..fbfb79713f23 100644 --- a/slirp/ncsi.c +++ b/slirp/ncsi.c @@ -8,9 +8,23 @@ */ #include "qemu/osdep.h" #include "slirp.h" +#include "net/checksum.h" #include "ncsi-pkt.h" +static uint32_t ncsi_calculate_checksum(unsigned char *data, int len) +{ + uint32_t checksum = 0; + int i; + + for (i = 0; i < len; i += 2) { + checksum += (((uint32_t)data[i] << 8) | data[i + 1]); + } + + checksum = (~checksum + 1); + return checksum; +} + /* Get Capabilities */ static int ncsi_rsp_handler_gc(struct ncsi_rsp_pkt_hdr *rnh) { @@ -108,6 +122,9 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) (ncsi_reply + ETH_HLEN); const struct ncsi_rsp_handler *handler = NULL; int i; + int ncsi_rsp_len = sizeof(*nh); + uint32_t checksum; + uint32_t *pchecksum; memset(ncsi_reply, 0, sizeof(ncsi_reply)); @@ -137,15 +154,19 @@ void ncsi_input(Slirp *slirp, const uint8_t *pkt, int pkt_len) /* TODO: handle errors */ handler->handler(rnh); } + ncsi_rsp_len += handler->payload; } else { rnh->common.length = 0; rnh->code = htons(NCSI_PKT_RSP_C_UNAVAILABLE); rnh->reason = htons(NCSI_PKT_RSP_R_UNKNOWN); } - /* TODO: add a checksum at the end of the frame but the specs - * allows it to be zero */ + /* add a checksum at the end of the frame (the specs allows it to + * be zero */ + checksum = ncsi_calculate_checksum((unsigned char *) rnh, ncsi_rsp_len); + pchecksum = (uint32_t *)((void *) rnh + ncsi_rsp_len); + *pchecksum = htonl(checksum); + ncsi_rsp_len += 4; - slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + sizeof(*nh) + - (handler ? handler->payload : 0) + 4); + slirp_output(slirp->opaque, ncsi_reply, ETH_HLEN + ncsi_rsp_len); }
The checksum field of a NC-SI packet contains a value that may be included in each command and response. The verification is optional but the Linux driver does so when a non-zero value is provided. Let's extend the model to compute the checksum value and exercise a little more the Linux driver. See section "8.2.2.3 - 2's Complement Checksum Compensation" in the Network Controller Sideband Interface (NC-SI) Specification for more details. Signed-off-by: Cédric Le Goater <clg@kaod.org> --- slirp/ncsi.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)