| Message ID | 20221117165554.1773409-3-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | rtl8139: honor large send MSS value | expand |
On 17/11/22 17:55, Stefan Hajnoczi wrote: > There are two Tx Descriptor formats called mode 0 and mode 1. The mode > is determined by the Large Send bit. > > CP_TX_IPCS (bit 18) is defined in mode 1 but the code checks the bit > unconditionally. In mode 0 bit 18 is part of the Large Send MSS value. > > Explicitly check the Large Send bit to distinguish Tx command modes. > This avoids bugs where modes are confused. Note that I didn't find any > actual bugs aside from needlessly computing the IP checksum when the > Large Send bit is enabled. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- > hw/net/rtl8139.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
On Fri, Nov 18, 2022 at 12:56 AM Stefan Hajnoczi <stefanha@redhat.com> wrote: > > There are two Tx Descriptor formats called mode 0 and mode 1. The mode > is determined by the Large Send bit. > > CP_TX_IPCS (bit 18) is defined in mode 1 but the code checks the bit > unconditionally. In mode 0 bit 18 is part of the Large Send MSS value. > > Explicitly check the Large Send bit to distinguish Tx command modes. > This avoids bugs where modes are confused. Note that I didn't find any > actual bugs aside from needlessly computing the IP checksum when the > Large Send bit is enabled. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Jason Wang <jasowang@redhat.com> Thanks > --- > hw/net/rtl8139.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c > index ffef3789b5..6dd7a8e6e0 100644 > --- a/hw/net/rtl8139.c > +++ b/hw/net/rtl8139.c > @@ -2135,7 +2135,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) > } > ip_data_len -= hlen; > > - if (txdw0 & CP_TX_IPCS) > + if (!(txdw0 & CP_TX_LGSEN) && (txdw0 & CP_TX_IPCS)) > { > DPRINTF("+++ C+ mode need IP checksum\n"); > > @@ -2268,7 +2268,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) > /* Stop sending this frame */ > saved_size = 0; > } > - else if (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS)) > + else if (!(txdw0 & CP_TX_LGSEN) && (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS))) > { > DPRINTF("+++ C+ mode need TCP or UDP checksum\n"); > > -- > 2.38.1 >
diff --git a/hw/net/rtl8139.c b/hw/net/rtl8139.c index ffef3789b5..6dd7a8e6e0 100644 --- a/hw/net/rtl8139.c +++ b/hw/net/rtl8139.c @@ -2135,7 +2135,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) } ip_data_len -= hlen; - if (txdw0 & CP_TX_IPCS) + if (!(txdw0 & CP_TX_LGSEN) && (txdw0 & CP_TX_IPCS)) { DPRINTF("+++ C+ mode need IP checksum\n"); @@ -2268,7 +2268,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s) /* Stop sending this frame */ saved_size = 0; } - else if (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS)) + else if (!(txdw0 & CP_TX_LGSEN) && (txdw0 & (CP_TX_TCPCS|CP_TX_UDPCS))) { DPRINTF("+++ C+ mode need TCP or UDP checksum\n");
There are two Tx Descriptor formats called mode 0 and mode 1. The mode is determined by the Large Send bit. CP_TX_IPCS (bit 18) is defined in mode 1 but the code checks the bit unconditionally. In mode 0 bit 18 is part of the Large Send MSS value. Explicitly check the Large Send bit to distinguish Tx command modes. This avoids bugs where modes are confused. Note that I didn't find any actual bugs aside from needlessly computing the IP checksum when the Large Send bit is enabled. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/net/rtl8139.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)