* [PATCH v3 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 2/9] igb: handle PF/VF reset properly Sriram Yagnaraman
` (7 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
I would like to review and be informed on changes to igb device
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index ece23b2b15..7d0e84ce37 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2224,6 +2224,7 @@ F: tests/qtest/libqos/e1000e.*
igb
M: Akihiko Odaki <akihiko.odaki@daynix.com>
+R: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
S: Maintained
F: docs/system/devices/igb.rst
F: hw/net/igb*
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 2/9] igb: handle PF/VF reset properly
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 3/9] igb: add ICR_RXDW Sriram Yagnaraman
` (6 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Use PFRSTD to reset RSTI bit for VFs, and raise VFLRE interrupt when VF
is reset.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 33 +++++++++++++++++++++------------
hw/net/igb_regs.h | 3 +++
hw/net/trace-events | 2 ++
3 files changed, 26 insertions(+), 12 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index bf1bcac747..9c32ad5e36 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1904,14 +1904,6 @@ static void igb_set_eims(IGBCore *core, int index, uint32_t val)
igb_update_interrupt_state(core);
}
-static void igb_vf_reset(IGBCore *core, uint16_t vfn)
-{
- /* TODO: Reset of the queue enable and the interrupt registers of the VF. */
-
- core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
- core->mac[V2PMAILBOX0 + vfn] = E1000_V2PMAILBOX_RSTD;
-}
-
static void mailbox_interrupt_to_vf(IGBCore *core, uint16_t vfn)
{
uint32_t ent = core->mac[VTIVAR_MISC + vfn];
@@ -1989,6 +1981,17 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
}
}
+static void igb_vf_reset(IGBCore *core, uint16_t vfn)
+{
+ /* disable Rx and Tx for the VF*/
+ core->mac[VFTE] &= ~BIT(vfn);
+ core->mac[VFRE] &= ~BIT(vfn);
+ /* indicate VF reset to PF */
+ core->mac[VFLRE] |= BIT(vfn);
+ /* VFLRE and mailbox use the same interrupt cause */
+ mailbox_interrupt_to_pf(core);
+}
+
static void igb_w1c(IGBCore *core, int index, uint32_t val)
{
core->mac[index] &= ~val;
@@ -2243,14 +2246,20 @@ igb_set_status(IGBCore *core, int index, uint32_t val)
static void
igb_set_ctrlext(IGBCore *core, int index, uint32_t val)
{
- trace_e1000e_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
- !!(val & E1000_CTRL_EXT_SPD_BYPS));
-
- /* TODO: PFRSTD */
+ trace_igb_link_set_ext_params(!!(val & E1000_CTRL_EXT_ASDCHK),
+ !!(val & E1000_CTRL_EXT_SPD_BYPS),
+ !!(val & E1000_CTRL_EXT_PFRSTD));
/* Zero self-clearing bits */
val &= ~(E1000_CTRL_EXT_ASDCHK | E1000_CTRL_EXT_EE_RST);
core->mac[CTRL_EXT] = val;
+
+ if (core->mac[CTRL_EXT] & E1000_CTRL_EXT_PFRSTD) {
+ for (int vfn = 0; vfn < IGB_MAX_VF_FUNCTIONS; vfn++) {
+ core->mac[V2PMAILBOX0 + vfn] &= ~E1000_V2PMAILBOX_RSTI;
+ core->mac[V2PMAILBOX0 + vfn] |= E1000_V2PMAILBOX_RSTD;
+ }
+ }
}
static void
diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h
index ebf3e95023..ddc0f931d6 100644
--- a/hw/net/igb_regs.h
+++ b/hw/net/igb_regs.h
@@ -240,6 +240,9 @@ union e1000_adv_rx_desc {
/* from igb/e1000_defines.h */
+/* Physical Func Reset Done Indication */
+#define E1000_CTRL_EXT_PFRSTD 0x00004000
+
#define E1000_IVAR_VALID 0x80
#define E1000_GPIE_NSICR 0x00000001
#define E1000_GPIE_MSIX_MODE 0x00000010
diff --git a/hw/net/trace-events b/hw/net/trace-events
index 2f791b9b57..e94172e748 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -281,6 +281,8 @@ igb_core_mdic_read_unhandled(uint32_t addr) "MDIC READ: PHY[%u] UNHANDLED"
igb_core_mdic_write(uint32_t addr, uint32_t data) "MDIC WRITE: PHY[%u] = 0x%x"
igb_core_mdic_write_unhandled(uint32_t addr) "MDIC WRITE: PHY[%u] UNHANDLED"
+igb_link_set_ext_params(bool asd_check, bool speed_select_bypass, bool pfrstd) "Set extended link params: ASD check: %d, Speed select bypass: %d, PF reset done: %d"
+
igb_rx_desc_buff_size(uint32_t b) "buffer size: %u"
igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint32_t len) "addr: 0x%"PRIx64", offset: %u, from: %p, length: %u"
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 3/9] igb: add ICR_RXDW
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 1/9] MAINTAINERS: Add Sriram Yagnaraman as a igb reviewer Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 2/9] igb: handle PF/VF reset properly Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-02-01 4:35 ` Akihiko Odaki
2023-01-31 9:42 ` [PATCH v3 4/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
` (5 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
IGB uses RXDW ICR bit to indicate that rx descriptor has been written
back. This is the same as RXT0 bit in older HW.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/e1000x_regs.h | 4 ++++
hw/net/igb_core.c | 46 +++++++++++++++++---------------------------
2 files changed, 22 insertions(+), 28 deletions(-)
diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
index fb5b861135..f509db73a7 100644
--- a/hw/net/e1000x_regs.h
+++ b/hw/net/e1000x_regs.h
@@ -335,6 +335,7 @@
#define E1000_ICR_RXDMT0 0x00000010 /* rx desc min. threshold (0) */
#define E1000_ICR_RXO 0x00000040 /* rx overrun */
#define E1000_ICR_RXT0 0x00000080 /* rx timer intr (ring 0) */
+#define E1000_ICR_RXDW 0x00000080 /* rx desc written back */
#define E1000_ICR_MDAC 0x00000200 /* MDIO access complete */
#define E1000_ICR_RXCFG 0x00000400 /* RX /c/ ordered set */
#define E1000_ICR_GPI_EN0 0x00000800 /* GP Int 0 */
@@ -378,6 +379,7 @@
#define E1000_ICS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_ICS_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_ICS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_ICS_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_ICS_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_ICS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_ICS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
@@ -407,6 +409,7 @@
#define E1000_IMS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_IMS_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_IMS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_IMS_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_IMS_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_IMS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_IMS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
@@ -441,6 +444,7 @@
#define E1000_IMC_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
#define E1000_IMC_RXO E1000_ICR_RXO /* rx overrun */
#define E1000_IMC_RXT0 E1000_ICR_RXT0 /* rx timer intr */
+#define E1000_IMC_RXDW E1000_ICR_RXDW /* rx desc written back */
#define E1000_IMC_MDAC E1000_ICR_MDAC /* MDIO access complete */
#define E1000_IMC_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
#define E1000_IMC_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 9c32ad5e36..e78bc3611a 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1488,7 +1488,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
uint16_t queues = 0;
- uint32_t n;
+ uint32_t icr_bits = 0;
uint8_t min_buf[ETH_ZLEN];
struct iovec min_iov;
struct eth_header *ehdr;
@@ -1561,6 +1561,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
e1000x_fcs_len(core->mac);
retval = orig_size;
+ igb_rx_fix_l4_csum(core, core->rx_pkt);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
if (!(queues & BIT(i))) {
@@ -1569,43 +1570,32 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
igb_rx_ring_init(core, &rxr, i);
- trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
-
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
- retval = 0;
+ icr_bits |= E1000_ICS_RXO;
+ continue;
}
- }
- if (retval) {
- n = E1000_ICR_RXT0;
-
- igb_rx_fix_l4_csum(core, core->rx_pkt);
-
- for (i = 0; i < IGB_NUM_QUEUES; i++) {
- if (!(queues & BIT(i))) {
- continue;
- }
-
- igb_rx_ring_init(core, &rxr, i);
+ trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
+ igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
- igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
+ /* Check if receive descriptor minimum threshold hit */
+ if (igb_rx_descr_threshold_hit(core, rxr.i)) {
+ icr_bits |= E1000_ICS_RXDMT0;
+ }
- /* Check if receive descriptor minimum threshold hit */
- if (igb_rx_descr_threshold_hit(core, rxr.i)) {
- n |= E1000_ICS_RXDMT0;
- }
+ core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
- core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
- }
+ icr_bits |= E1000_ICR_RXDW;
+ }
- trace_e1000e_rx_written_to_guest(n);
+ if (icr_bits & E1000_ICR_RXDW) {
+ trace_e1000e_rx_written_to_guest(icr_bits);
} else {
- n = E1000_ICS_RXO;
- trace_e1000e_rx_not_written_to_guest(n);
+ trace_e1000e_rx_not_written_to_guest(icr_bits);
}
- trace_e1000e_rx_interrupt_set(n);
- igb_set_interrupt_cause(core, n);
+ trace_e1000e_rx_interrupt_set(icr_bits);
+ igb_set_interrupt_cause(core, icr_bits);
return retval;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 3/9] igb: add ICR_RXDW
2023-01-31 9:42 ` [PATCH v3 3/9] igb: add ICR_RXDW Sriram Yagnaraman
@ 2023-02-01 4:35 ` Akihiko Odaki
2023-02-01 10:29 ` Sriram Yagnaraman
0 siblings, 1 reply; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 4:35 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> IGB uses RXDW ICR bit to indicate that rx descriptor has been written
> back. This is the same as RXT0 bit in older HW.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/e1000x_regs.h | 4 ++++
> hw/net/igb_core.c | 46 +++++++++++++++++---------------------------
> 2 files changed, 22 insertions(+), 28 deletions(-)
>
> diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h
> index fb5b861135..f509db73a7 100644
> --- a/hw/net/e1000x_regs.h
> +++ b/hw/net/e1000x_regs.h
> @@ -335,6 +335,7 @@
> #define E1000_ICR_RXDMT0 0x00000010 /* rx desc min. threshold (0) */
> #define E1000_ICR_RXO 0x00000040 /* rx overrun */
> #define E1000_ICR_RXT0 0x00000080 /* rx timer intr (ring 0) */
> +#define E1000_ICR_RXDW 0x00000080 /* rx desc written back */
> #define E1000_ICR_MDAC 0x00000200 /* MDIO access complete */
> #define E1000_ICR_RXCFG 0x00000400 /* RX /c/ ordered set */
> #define E1000_ICR_GPI_EN0 0x00000800 /* GP Int 0 */
> @@ -378,6 +379,7 @@
> #define E1000_ICS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
> #define E1000_ICS_RXO E1000_ICR_RXO /* rx overrun */
> #define E1000_ICS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> +#define E1000_ICS_RXDW E1000_ICR_RXDW /* rx desc written back */
> #define E1000_ICS_MDAC E1000_ICR_MDAC /* MDIO access complete */
> #define E1000_ICS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> #define E1000_ICS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> @@ -407,6 +409,7 @@
> #define E1000_IMS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
> #define E1000_IMS_RXO E1000_ICR_RXO /* rx overrun */
> #define E1000_IMS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> +#define E1000_IMS_RXDW E1000_ICR_RXDW /* rx desc written back */
> #define E1000_IMS_MDAC E1000_ICR_MDAC /* MDIO access complete */
> #define E1000_IMS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> #define E1000_IMS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> @@ -441,6 +444,7 @@
> #define E1000_IMC_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min. threshold */
> #define E1000_IMC_RXO E1000_ICR_RXO /* rx overrun */
> #define E1000_IMC_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> +#define E1000_IMC_RXDW E1000_ICR_RXDW /* rx desc written back */
> #define E1000_IMC_MDAC E1000_ICR_MDAC /* MDIO access complete */
> #define E1000_IMC_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> #define E1000_IMC_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 9c32ad5e36..e78bc3611a 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -1488,7 +1488,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
>
> uint16_t queues = 0;
> - uint32_t n;
> + uint32_t icr_bits = 0;
> uint8_t min_buf[ETH_ZLEN];
> struct iovec min_iov;
> struct eth_header *ehdr;
> @@ -1561,6 +1561,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> e1000x_fcs_len(core->mac);
>
> retval = orig_size;
> + igb_rx_fix_l4_csum(core, core->rx_pkt);
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> if (!(queues & BIT(i))) {
> @@ -1569,43 +1570,32 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>
> igb_rx_ring_init(core, &rxr, i);
>
> - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> -
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> - retval = 0;
> + icr_bits |= E1000_ICS_RXO;
> + continue;
> }
> - }
>
> - if (retval) {
> - n = E1000_ICR_RXT0;
> -
> - igb_rx_fix_l4_csum(core, core->rx_pkt);
> -
> - for (i = 0; i < IGB_NUM_QUEUES; i++) {
> - if (!(queues & BIT(i))) {
> - continue;
> - }
> -
> - igb_rx_ring_init(core, &rxr, i);
> + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> + igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
>
> - igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
> + /* Check if receive descriptor minimum threshold hit */
> + if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> + icr_bits |= E1000_ICS_RXDMT0;
> + }
>
> - /* Check if receive descriptor minimum threshold hit */
> - if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> - n |= E1000_ICS_RXDMT0;
> - }
> + core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
>
> - core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> - }
> + icr_bits |= E1000_ICR_RXDW;
> + }
>
> - trace_e1000e_rx_written_to_guest(n);
> + if (icr_bits & E1000_ICR_RXDW) {
> + trace_e1000e_rx_written_to_guest(icr_bits);
> } else {
> - n = E1000_ICS_RXO;
> - trace_e1000e_rx_not_written_to_guest(n);
> + trace_e1000e_rx_not_written_to_guest(icr_bits);
> }
>
> - trace_e1000e_rx_interrupt_set(n);
> - igb_set_interrupt_cause(core, n);
> + trace_e1000e_rx_interrupt_set(icr_bits);
> + igb_set_interrupt_cause(core, icr_bits);
>
> return retval;
> }
The change for igb_receive_internal() actually fixes a bug; even if a
pool doesn't have enough space to write back a packet, it shouldn't
prevent other pools from receiving the packet.
I have fixed in v7 (well, I intended to do that in v6 but I made some
mistakes) of "introduce igb" series, but there are some differences:
- "n" is not renamed to "icr_bits". Yes, "n" is a bad name, but if it's
to be fixed, it should be done for e1000e too at the same time.
- "retval" variable is removed.
- tracepoints were updated so that we can see to which queue the Rx
packet is written back.
E1000_ICR_RXDW is still not added to "introduce igb" series so please
rebase this and submit again.
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v3 3/9] igb: add ICR_RXDW
2023-02-01 4:35 ` Akihiko Odaki
@ 2023-02-01 10:29 ` Sriram Yagnaraman
0 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-02-01 10:29 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel@nongnu.org, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Wednesday, 1 February 2023 05:36
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH v3 3/9] igb: add ICR_RXDW
>
> On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> > IGB uses RXDW ICR bit to indicate that rx descriptor has been written
> > back. This is the same as RXT0 bit in older HW.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > ---
> > hw/net/e1000x_regs.h | 4 ++++
> > hw/net/igb_core.c | 46 +++++++++++++++++---------------------------
> > 2 files changed, 22 insertions(+), 28 deletions(-)
> >
> > diff --git a/hw/net/e1000x_regs.h b/hw/net/e1000x_regs.h index
> > fb5b861135..f509db73a7 100644
> > --- a/hw/net/e1000x_regs.h
> > +++ b/hw/net/e1000x_regs.h
> > @@ -335,6 +335,7 @@
> > #define E1000_ICR_RXDMT0 0x00000010 /* rx desc min. threshold (0)
> */
> > #define E1000_ICR_RXO 0x00000040 /* rx overrun */
> > #define E1000_ICR_RXT0 0x00000080 /* rx timer intr (ring 0) */
> > +#define E1000_ICR_RXDW 0x00000080 /* rx desc written back */
> > #define E1000_ICR_MDAC 0x00000200 /* MDIO access complete */
> > #define E1000_ICR_RXCFG 0x00000400 /* RX /c/ ordered set */
> > #define E1000_ICR_GPI_EN0 0x00000800 /* GP Int 0 */
> > @@ -378,6 +379,7 @@
> > #define E1000_ICS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min.
> threshold */
> > #define E1000_ICS_RXO E1000_ICR_RXO /* rx overrun */
> > #define E1000_ICS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> > +#define E1000_ICS_RXDW E1000_ICR_RXDW /* rx desc written back
> */
> > #define E1000_ICS_MDAC E1000_ICR_MDAC /* MDIO access
> complete */
> > #define E1000_ICS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> > #define E1000_ICS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> > @@ -407,6 +409,7 @@
> > #define E1000_IMS_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min.
> threshold */
> > #define E1000_IMS_RXO E1000_ICR_RXO /* rx overrun */
> > #define E1000_IMS_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> > +#define E1000_IMS_RXDW E1000_ICR_RXDW /* rx desc written back
> */
> > #define E1000_IMS_MDAC E1000_ICR_MDAC /* MDIO access
> complete */
> > #define E1000_IMS_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> > #define E1000_IMS_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> > @@ -441,6 +444,7 @@
> > #define E1000_IMC_RXDMT0 E1000_ICR_RXDMT0 /* rx desc min.
> threshold */
> > #define E1000_IMC_RXO E1000_ICR_RXO /* rx overrun */
> > #define E1000_IMC_RXT0 E1000_ICR_RXT0 /* rx timer intr */
> > +#define E1000_IMC_RXDW E1000_ICR_RXDW /* rx desc written back
> */
> > #define E1000_IMC_MDAC E1000_ICR_MDAC /* MDIO access
> complete */
> > #define E1000_IMC_RXCFG E1000_ICR_RXCFG /* RX /c/ ordered set */
> > #define E1000_IMC_GPI_EN0 E1000_ICR_GPI_EN0 /* GP Int 0 */
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > 9c32ad5e36..e78bc3611a 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -1488,7 +1488,7 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
> > static const int maximum_ethernet_hdr_len = (ETH_HLEN + 4);
> >
> > uint16_t queues = 0;
> > - uint32_t n;
> > + uint32_t icr_bits = 0;
> > uint8_t min_buf[ETH_ZLEN];
> > struct iovec min_iov;
> > struct eth_header *ehdr;
> > @@ -1561,6 +1561,7 @@ igb_receive_internal(IGBCore *core, const struct
> iovec *iov, int iovcnt,
> > e1000x_fcs_len(core->mac);
> >
> > retval = orig_size;
> > + igb_rx_fix_l4_csum(core, core->rx_pkt);
> >
> > for (i = 0; i < IGB_NUM_QUEUES; i++) {
> > if (!(queues & BIT(i))) {
> > @@ -1569,43 +1570,32 @@ igb_receive_internal(IGBCore *core, const
> > struct iovec *iov, int iovcnt,
> >
> > igb_rx_ring_init(core, &rxr, i);
> >
> > - trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> > -
> > if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> > - retval = 0;
> > + icr_bits |= E1000_ICS_RXO;
> > + continue;
> > }
> > - }
> >
> > - if (retval) {
> > - n = E1000_ICR_RXT0;
> > -
> > - igb_rx_fix_l4_csum(core, core->rx_pkt);
> > -
> > - for (i = 0; i < IGB_NUM_QUEUES; i++) {
> > - if (!(queues & BIT(i))) {
> > - continue;
> > - }
> > -
> > - igb_rx_ring_init(core, &rxr, i);
> > + trace_e1000e_rx_rss_dispatched_to_queue(rxr.i->idx);
> > + igb_write_packet_to_guest(core, core->rx_pkt, &rxr,
> > + &rss_info);
> >
> > - igb_write_packet_to_guest(core, core->rx_pkt, &rxr, &rss_info);
> > + /* Check if receive descriptor minimum threshold hit */
> > + if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> > + icr_bits |= E1000_ICS_RXDMT0;
> > + }
> >
> > - /* Check if receive descriptor minimum threshold hit */
> > - if (igb_rx_descr_threshold_hit(core, rxr.i)) {
> > - n |= E1000_ICS_RXDMT0;
> > - }
> > + core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> >
> > - core->mac[EICR] |= igb_rx_wb_eic(core, rxr.i->idx);
> > - }
> > + icr_bits |= E1000_ICR_RXDW;
> > + }
> >
> > - trace_e1000e_rx_written_to_guest(n);
> > + if (icr_bits & E1000_ICR_RXDW) {
> > + trace_e1000e_rx_written_to_guest(icr_bits);
> > } else {
> > - n = E1000_ICS_RXO;
> > - trace_e1000e_rx_not_written_to_guest(n);
> > + trace_e1000e_rx_not_written_to_guest(icr_bits);
> > }
> >
> > - trace_e1000e_rx_interrupt_set(n);
> > - igb_set_interrupt_cause(core, n);
> > + trace_e1000e_rx_interrupt_set(icr_bits);
> > + igb_set_interrupt_cause(core, icr_bits);
> >
> > return retval;
> > }
>
> The change for igb_receive_internal() actually fixes a bug; even if a pool
> doesn't have enough space to write back a packet, it shouldn't prevent other
> pools from receiving the packet.
>
> I have fixed in v7 (well, I intended to do that in v6 but I made some
> mistakes) of "introduce igb" series, but there are some differences:
> - "n" is not renamed to "icr_bits". Yes, "n" is a bad name, but if it's to be fixed,
> it should be done for e1000e too at the same time.
> - "retval" variable is removed.
> - tracepoints were updated so that we can see to which queue the Rx packet is
> written back.
>
> E1000_ICR_RXDW is still not added to "introduce igb" series so please rebase
> this and submit again.
Okay, I fix that.
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 4/9] igb: implement VFRE and VFTE registers
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (2 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 3/9] igb: add ICR_RXDW Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-02-01 4:42 ` Akihiko Odaki
2023-01-31 9:42 ` [PATCH v3 5/9] igb: check oversized packets for VMDq Sriram Yagnaraman
` (4 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Also add checks for RXDCTL/TXDCTL queue enable bits
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 30 +++++++++++++++++++++++++-----
hw/net/igb_core.h | 1 +
hw/net/igb_regs.h | 3 +++
3 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index e78bc3611a..4a1b98bf0e 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -780,6 +780,18 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t base,
return igb_tx_wb_eic(core, txi->idx);
}
+static inline bool
+igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi)
+{
+ bool vmdq = core->mac[MRQC] & 1;
+ uint16_t qn = txi->idx;
+ uint16_t pool = qn % IGB_NUM_VM_POOLS;
+
+ return (core->mac[TCTL] & E1000_TCTL_EN) &&
+ (!vmdq || core->mac[VFTE] & BIT(pool)) &&
+ (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
+}
+
static void
igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
{
@@ -789,8 +801,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
const E1000E_RingInfo *txi = txr->i;
uint32_t eic = 0;
- /* TODO: check if the queue itself is enabled too. */
- if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
+ if (!igb_tx_enabled(core, txi)) {
trace_e1000e_tx_disabled();
return;
}
@@ -1005,6 +1016,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
}
+ queues &= core->mac[VFRE];
igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
if (rss_info->queue & 1) {
queues <<= 8;
@@ -1564,12 +1576,12 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
igb_rx_fix_l4_csum(core, core->rx_pkt);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
- if (!(queues & BIT(i))) {
+ if (!(queues & BIT(i)) ||
+ !(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
continue;
}
igb_rx_ring_init(core, &rxr, i);
-
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
icr_bits |= E1000_ICS_RXO;
continue;
@@ -1973,9 +1985,16 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
static void igb_vf_reset(IGBCore *core, uint16_t vfn)
{
+ uint16_t qn0 = vfn;
+ uint16_t qn1 = vfn + IGB_NUM_VM_POOLS;
+
/* disable Rx and Tx for the VF*/
- core->mac[VFTE] &= ~BIT(vfn);
+ core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
+ core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
+ core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
+ core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
core->mac[VFRE] &= ~BIT(vfn);
+ core->mac[VFTE] &= ~BIT(vfn);
/* indicate VF reset to PF */
core->mac[VFLRE] |= BIT(vfn);
/* VFLRE and mailbox use the same interrupt cause */
@@ -3881,6 +3900,7 @@ igb_phy_reg_init[] = {
static const uint32_t igb_mac_reg_init[] = {
[LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
[EEMNGCTL] = BIT(31),
+ [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
[RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
[RXDCTL1] = 1 << 16,
[RXDCTL2] = 1 << 16,
diff --git a/hw/net/igb_core.h b/hw/net/igb_core.h
index cc3b4d1f2b..9938922598 100644
--- a/hw/net/igb_core.h
+++ b/hw/net/igb_core.h
@@ -47,6 +47,7 @@
#define IGB_MSIX_VEC_NUM (10)
#define IGBVF_MSIX_VEC_NUM (3)
#define IGB_NUM_QUEUES (16)
+#define IGB_NUM_VM_POOLS (8)
typedef struct IGBCore IGBCore;
diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h
index ddc0f931d6..4d98079906 100644
--- a/hw/net/igb_regs.h
+++ b/hw/net/igb_regs.h
@@ -160,6 +160,9 @@ union e1000_adv_rx_desc {
#define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
#define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
+/* Additional Transmit Descriptor Control definitions */
+#define E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue */
+
/* Additional Receive Descriptor Control definitions */
#define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx Queue */
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 4/9] igb: implement VFRE and VFTE registers
2023-01-31 9:42 ` [PATCH v3 4/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
@ 2023-02-01 4:42 ` Akihiko Odaki
0 siblings, 0 replies; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 4:42 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> Also add checks for RXDCTL/TXDCTL queue enable bits
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 30 +++++++++++++++++++++++++-----
> hw/net/igb_core.h | 1 +
> hw/net/igb_regs.h | 3 +++
> 3 files changed, 29 insertions(+), 5 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index e78bc3611a..4a1b98bf0e 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -780,6 +780,18 @@ igb_txdesc_writeback(IGBCore *core, dma_addr_t base,
> return igb_tx_wb_eic(core, txi->idx);
> }
>
> +static inline bool
> +igb_tx_enabled(IGBCore *core, const E1000E_RingInfo *txi)
> +{
> + bool vmdq = core->mac[MRQC] & 1;
> + uint16_t qn = txi->idx;
> + uint16_t pool = qn % IGB_NUM_VM_POOLS;
> +
> + return (core->mac[TCTL] & E1000_TCTL_EN) &&
> + (!vmdq || core->mac[VFTE] & BIT(pool)) &&
> + (core->mac[TXDCTL0 + (qn * 16)] & E1000_TXDCTL_QUEUE_ENABLE);
> +}
> +
> static void
> igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
> {
> @@ -789,8 +801,7 @@ igb_start_xmit(IGBCore *core, const IGB_TxRing *txr)
> const E1000E_RingInfo *txi = txr->i;
> uint32_t eic = 0;
>
> - /* TODO: check if the queue itself is enabled too. */
> - if (!(core->mac[TCTL] & E1000_TCTL_EN)) {
> + if (!igb_tx_enabled(core, txi)) {
> trace_e1000e_tx_disabled();
> return;
> }
> @@ -1005,6 +1016,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> queues = BIT(def_pl >> E1000_VT_CTL_DEFAULT_POOL_SHIFT);
> }
>
> + queues &= core->mac[VFRE];
> igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
> if (rss_info->queue & 1) {
> queues <<= 8;
> @@ -1564,12 +1576,12 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> igb_rx_fix_l4_csum(core, core->rx_pkt);
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> - if (!(queues & BIT(i))) {
> + if (!(queues & BIT(i)) ||
> + !(core->mac[RXDCTL0 + (i * 16)] & E1000_RXDCTL_QUEUE_ENABLE)) {
> continue;
> }
>
> igb_rx_ring_init(core, &rxr, i);
> -
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> icr_bits |= E1000_ICS_RXO;
> continue;
> @@ -1973,9 +1985,16 @@ static void igb_set_vfmailbox(IGBCore *core, int index, uint32_t val)
>
> static void igb_vf_reset(IGBCore *core, uint16_t vfn)
> {
> + uint16_t qn0 = vfn;
> + uint16_t qn1 = vfn + IGB_NUM_VM_POOLS;
> +
> /* disable Rx and Tx for the VF*/
> - core->mac[VFTE] &= ~BIT(vfn);
> + core->mac[RXDCTL0 + (qn0 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> + core->mac[RXDCTL0 + (qn1 * 16)] &= ~E1000_RXDCTL_QUEUE_ENABLE;
> + core->mac[TXDCTL0 + (qn0 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> + core->mac[TXDCTL0 + (qn1 * 16)] &= ~E1000_TXDCTL_QUEUE_ENABLE;
> core->mac[VFRE] &= ~BIT(vfn);
> + core->mac[VFTE] &= ~BIT(vfn);
> /* indicate VF reset to PF */
> core->mac[VFLRE] |= BIT(vfn);
> /* VFLRE and mailbox use the same interrupt cause */
> @@ -3881,6 +3900,7 @@ igb_phy_reg_init[] = {
> static const uint32_t igb_mac_reg_init[] = {
> [LEDCTL] = 2 | (3 << 8) | BIT(15) | (6 << 16) | (7 << 24),
> [EEMNGCTL] = BIT(31),
> + [TXDCTL0] = E1000_TXDCTL_QUEUE_ENABLE,
> [RXDCTL0] = E1000_RXDCTL_QUEUE_ENABLE | (1 << 16),
> [RXDCTL1] = 1 << 16,
> [RXDCTL2] = 1 << 16,
> diff --git a/hw/net/igb_core.h b/hw/net/igb_core.h
> index cc3b4d1f2b..9938922598 100644
> --- a/hw/net/igb_core.h
> +++ b/hw/net/igb_core.h
> @@ -47,6 +47,7 @@
> #define IGB_MSIX_VEC_NUM (10)
> #define IGBVF_MSIX_VEC_NUM (3)
> #define IGB_NUM_QUEUES (16)
> +#define IGB_NUM_VM_POOLS (8)
If you are adding this definition, search for "8" and replace the
occurrences with it where appropriate.
>
> typedef struct IGBCore IGBCore;
>
> diff --git a/hw/net/igb_regs.h b/hw/net/igb_regs.h
> index ddc0f931d6..4d98079906 100644
> --- a/hw/net/igb_regs.h
> +++ b/hw/net/igb_regs.h
> @@ -160,6 +160,9 @@ union e1000_adv_rx_desc {
> #define E1000_MRQC_RSS_FIELD_IPV6_UDP 0x00800000
> #define E1000_MRQC_RSS_FIELD_IPV6_UDP_EX 0x01000000
>
> +/* Additional Transmit Descriptor Control definitions */
> +#define E1000_TXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Tx Queue */
> +
> /* Additional Receive Descriptor Control definitions */
> #define E1000_RXDCTL_QUEUE_ENABLE 0x02000000 /* Enable specific Rx Queue */
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 5/9] igb: check oversized packets for VMDq
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (3 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 4/9] igb: implement VFRE and VFTE registers Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-02-01 4:39 ` Akihiko Odaki
2023-01-31 9:42 ` [PATCH v3 6/9] igb: respect E1000_VMOLR_RSSE Sriram Yagnaraman
` (3 subsequent siblings)
8 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 48 +++++++++++++++++++++++++++++++++++++++--------
1 file changed, 40 insertions(+), 8 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 4a1b98bf0e..2f6f30341f 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -912,12 +912,27 @@ igb_rx_l4_cso_enabled(IGBCore *core)
return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD);
}
+static bool
+igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
+{
+ uint16_t pool = qn % IGB_NUM_VM_POOLS;
+ bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
+ int maximum_ethernet_lpe_size =
+ core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
+ int maximum_ethernet_vlan_size = 1522;
+
+ return lpe ? size > maximum_ethernet_lpe_size :
+ size > maximum_ethernet_vlan_size;
+}
+
static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
- E1000E_RSSInfo *rss_info, bool *external_tx)
+ size_t size, E1000E_RSSInfo *rss_info,
+ bool *external_tx)
{
static const int ta_shift[] = { 4, 3, 2, 0 };
uint32_t f, ra[2], *macp, rctl = core->mac[RCTL];
uint16_t queues = 0;
+ uint16_t oversized = 0;
uint16_t vid = lduw_be_p(&PKT_GET_VLAN_HDR(ehdr)->h_tci) & VLAN_VID_MASK;
bool accepted = false;
int i;
@@ -943,7 +958,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
if (core->mac[MRQC] & 1) {
if (is_broadcast_ether_addr(ehdr->h_dest)) {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
queues |= BIT(i);
}
@@ -977,7 +992,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
if (macp[f >> 5] & (1 << (f & 0x1f))) {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
queues |= BIT(i);
}
@@ -1000,7 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
}
} else {
- for (i = 0; i < 8; i++) {
+ for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
mask |= BIT(i);
}
@@ -1017,9 +1032,26 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
queues &= core->mac[VFRE];
- igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
- if (rss_info->queue & 1) {
- queues <<= 8;
+ if (queues) {
+ for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
+ if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size)) {
+ oversized |= BIT(i);
+ }
+ }
+ /* 8.19.37 increment ROC if packet is oversized for all queues */
+ if (oversized == queues) {
+ trace_e1000x_rx_oversized(size);
+ e1000x_inc_reg_if_not_full(core->mac, ROC);
+ }
+ queues &= ~oversized;
+ }
+
+ if (queues) {
+ igb_rss_parse_packet(core, core->rx_pkt,
+ external_tx != NULL, rss_info);
+ if (rss_info->queue & 1) {
+ queues <<= 8;
+ }
}
} else {
switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
@@ -1563,7 +1595,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
e1000x_vlan_enabled(core->mac),
core->mac[VET] & 0xffff);
- queues = igb_receive_assign(core, ehdr, &rss_info, external_tx);
+ queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
if (!queues) {
trace_e1000e_rx_flt_dropped();
return orig_size;
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 5/9] igb: check oversized packets for VMDq
2023-01-31 9:42 ` [PATCH v3 5/9] igb: check oversized packets for VMDq Sriram Yagnaraman
@ 2023-02-01 4:39 ` Akihiko Odaki
0 siblings, 0 replies; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 4:39 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 48 +++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 40 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 4a1b98bf0e..2f6f30341f 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -912,12 +912,27 @@ igb_rx_l4_cso_enabled(IGBCore *core)
> return !!(core->mac[RXCSUM] & E1000_RXCSUM_TUOFLD);
> }
>
> +static bool
> +igb_rx_is_oversized(IGBCore *core, uint16_t qn, size_t size)
> +{
> + uint16_t pool = qn % IGB_NUM_VM_POOLS;
> + bool lpe = !!(core->mac[VMOLR0 + pool] & E1000_VMOLR_LPE);
> + int maximum_ethernet_lpe_size =
> + core->mac[VMOLR0 + pool] & E1000_VMOLR_RLPML_MASK;
> + int maximum_ethernet_vlan_size = 1522;
> +
> + return lpe ? size > maximum_ethernet_lpe_size :
> + size > maximum_ethernet_vlan_size;
Instead do:
size > (lpe ? maximum_ethernet_lpe_size : maximum_ethernet_vlan_size)
> +}
> +
> static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> - E1000E_RSSInfo *rss_info, bool *external_tx)
> + size_t size, E1000E_RSSInfo *rss_info,
> + bool *external_tx)
> {
> static const int ta_shift[] = { 4, 3, 2, 0 };
> uint32_t f, ra[2], *macp, rctl = core->mac[RCTL];
> uint16_t queues = 0;
> + uint16_t oversized = 0;
> uint16_t vid = lduw_be_p(&PKT_GET_VLAN_HDR(ehdr)->h_tci) & VLAN_VID_MASK;
> bool accepted = false;
> int i;
> @@ -943,7 +958,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
>
> if (core->mac[MRQC] & 1) {
> if (is_broadcast_ether_addr(ehdr->h_dest)) {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
> queues |= BIT(i);
> }
> @@ -977,7 +992,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> f = ta_shift[(rctl >> E1000_RCTL_MO_SHIFT) & 3];
> f = (((ehdr->h_dest[5] << 8) | ehdr->h_dest[4]) >> f) & 0xfff;
> if (macp[f >> 5] & (1 << (f & 0x1f))) {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_ROMPE) {
> queues |= BIT(i);
> }
> @@ -1000,7 +1015,7 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> }
> }
> } else {
> - for (i = 0; i < 8; i++) {
> + for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_AUPE) {
> mask |= BIT(i);
> }
> @@ -1017,9 +1032,26 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> }
>
> queues &= core->mac[VFRE];
> - igb_rss_parse_packet(core, core->rx_pkt, external_tx != NULL, rss_info);
> - if (rss_info->queue & 1) {
> - queues <<= 8;
> + if (queues) {
> + for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> + if ((queues & BIT(i)) && igb_rx_is_oversized(core, i, size)) {
> + oversized |= BIT(i);
> + }
> + }
> + /* 8.19.37 increment ROC if packet is oversized for all queues */
> + if (oversized == queues) {
> + trace_e1000x_rx_oversized(size);
> + e1000x_inc_reg_if_not_full(core->mac, ROC);
> + }
> + queues &= ~oversized;
> + }
> +
> + if (queues) {
> + igb_rss_parse_packet(core, core->rx_pkt,
> + external_tx != NULL, rss_info);
> + if (rss_info->queue & 1) {
> + queues <<= 8;
> + }
> }
> } else {
> switch (net_rx_pkt_get_packet_type(core->rx_pkt)) {
> @@ -1563,7 +1595,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> e1000x_vlan_enabled(core->mac),
> core->mac[VET] & 0xffff);
>
> - queues = igb_receive_assign(core, ehdr, &rss_info, external_tx);
> + queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
> if (!queues) {
> trace_e1000e_rx_flt_dropped();
> return orig_size;
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 6/9] igb: respect E1000_VMOLR_RSSE
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (4 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 5/9] igb: check oversized packets for VMDq Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 7/9] igb: implement VF Tx and Rx stats Sriram Yagnaraman
` (2 subsequent siblings)
8 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
RSS for VFs is only enabled if VMOLR[n].RSSE is set.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 2f6f30341f..31a325d374 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -1049,8 +1049,15 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
if (queues) {
igb_rss_parse_packet(core, core->rx_pkt,
external_tx != NULL, rss_info);
+ /* Sec 8.26.1: PQn = VFn + VQn*8 */
if (rss_info->queue & 1) {
- queues <<= 8;
+ for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
+ if ((queues & BIT(i)) &&
+ (core->mac[VMOLR0 + i] & E1000_VMOLR_RSSE)) {
+ queues |= BIT(i + IGB_NUM_VM_POOLS);
+ queues &= ~BIT(i);
+ }
+ }
}
}
} else {
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 7/9] igb: implement VF Tx and Rx stats
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (5 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 6/9] igb: respect E1000_VMOLR_RSSE Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field Sriram Yagnaraman
2023-01-31 9:42 ` [PATCH v3 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
8 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Please note that loopback counters for VM to VM traffic is not
implemented yet: VFGOTLBC, VFGPTLBC, VFGORLBC and VFGPRLBC.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 31a325d374..c5f9c14f47 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -490,7 +490,7 @@ igb_tx_pkt_send(IGBCore *core, struct igb_tx *tx, int queue_index)
}
static void
-igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt)
+igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt, int qn)
{
static const int PTCregs[6] = { PTC64, PTC127, PTC255, PTC511,
PTC1023, PTC1522 };
@@ -517,6 +517,13 @@ igb_on_tx_done_update_stats(IGBCore *core, struct NetTxPkt *tx_pkt)
core->mac[GPTC] = core->mac[TPT];
core->mac[GOTCL] = core->mac[TOTL];
core->mac[GOTCH] = core->mac[TOTH];
+
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = qn % IGB_NUM_VM_POOLS;
+
+ core->mac[PVFGOTC0 + (pool * 64)] += tot_len;
+ core->mac[PVFGPTC0 + (pool * 64)]++;
+ }
}
static void
@@ -579,7 +586,7 @@ igb_process_tx_desc(IGBCore *core,
core->mac[VET] & 0xffff);
}
if (igb_tx_pkt_send(core, tx, queue_index)) {
- igb_on_tx_done_update_stats(core, tx->tx_pkt);
+ igb_on_tx_done_update_stats(core, tx->tx_pkt, queue_index);
}
}
@@ -1396,7 +1403,8 @@ igb_write_to_rx_buffers(IGBCore *core,
}
static void
-igb_update_rx_stats(IGBCore *core, size_t data_size, size_t data_fcs_size)
+igb_update_rx_stats(IGBCore *core, const E1000E_RingInfo *rxi,
+ size_t data_size, size_t data_fcs_size)
{
e1000x_update_rx_total_stats(core->mac, data_size, data_fcs_size);
@@ -1412,6 +1420,16 @@ igb_update_rx_stats(IGBCore *core, size_t data_size, size_t data_fcs_size)
default:
break;
}
+
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = rxi->idx % IGB_NUM_VM_POOLS;
+
+ core->mac[PVFGORC0 + (pool * 64)] += data_size + 4;
+ core->mac[PVFGPRC0 + (pool * 64)]++;
+ if (net_rx_pkt_get_packet_type(core->rx_pkt) == ETH_PKT_MCAST) {
+ core->mac[PVFMPRC0 + (pool * 64)]++;
+ }
+ }
}
static inline bool
@@ -1513,7 +1531,7 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
} while (desc_offset < total_size);
- igb_update_rx_stats(core, size, total_size);
+ igb_update_rx_stats(core, rxi, size, total_size);
}
static inline void
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (6 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 7/9] igb: implement VF Tx and Rx stats Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-02-01 4:58 ` Akihiko Odaki
2023-01-31 9:42 ` [PATCH v3 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
8 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Also trace out a warning if replication mode is disabled, since we only
support replication mode enabled.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 9 +++++++++
hw/net/trace-events | 2 ++
2 files changed, 11 insertions(+)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index c5f9c14f47..8115be2d76 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
if (core->mac[MRQC] & 1) {
+ if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
+ trace_igb_rx_vmdq_replication_mode_disabled();
+ }
+
if (is_broadcast_ether_addr(ehdr->h_dest)) {
for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
@@ -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
}
}
+ /* assume a full pool list if IGMAC is set */
+ if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
+ queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
+ }
+
if (e1000x_vlan_rx_filter_enabled(core->mac)) {
uint16_t mask = 0;
diff --git a/hw/net/trace-events b/hw/net/trace-events
index e94172e748..9bc7658692 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint3
igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
+igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication mode enabled is supported"
+
igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR enabled"
igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr) "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
2023-01-31 9:42 ` [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field Sriram Yagnaraman
@ 2023-02-01 4:58 ` Akihiko Odaki
2023-02-01 10:29 ` Sriram Yagnaraman
0 siblings, 1 reply; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 4:58 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> Also trace out a warning if replication mode is disabled, since we only
> support replication mode enabled.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 9 +++++++++
> hw/net/trace-events | 2 ++
> 2 files changed, 11 insertions(+)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index c5f9c14f47..8115be2d76 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> }
>
> if (core->mac[MRQC] & 1) {
> + if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
> + trace_igb_rx_vmdq_replication_mode_disabled();
> + }
> +
> if (is_broadcast_ether_addr(ehdr->h_dest)) {
> for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) {
> @@ -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore *core, const struct eth_header *ehdr,
> }
> }
>
> + /* assume a full pool list if IGMAC is set */
> + if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
> + queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
> + }
> +
This overwrites "queues", but "external_tx" is not overwritten.
> if (e1000x_vlan_rx_filter_enabled(core->mac)) {
> uint16_t mask = 0;
>
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index e94172e748..9bc7658692 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t offset, const void* source, uint3
>
> igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
>
> +igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication mode enabled is supported"
> +
> igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR enabled"
> igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr) "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
> igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
2023-02-01 4:58 ` Akihiko Odaki
@ 2023-02-01 10:29 ` Sriram Yagnaraman
2023-02-01 13:03 ` Akihiko Odaki
0 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-02-01 10:29 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel@nongnu.org, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Wednesday, 1 February 2023 05:58
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
>
> On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> > Also trace out a warning if replication mode is disabled, since we
> > only support replication mode enabled.
> >
> > Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> > ---
> > hw/net/igb_core.c | 9 +++++++++
> > hw/net/trace-events | 2 ++
> > 2 files changed, 11 insertions(+)
> >
> > diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> > c5f9c14f47..8115be2d76 100644
> > --- a/hw/net/igb_core.c
> > +++ b/hw/net/igb_core.c
> > @@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> > }
> >
> > if (core->mac[MRQC] & 1) {
> > + if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
> > + trace_igb_rx_vmdq_replication_mode_disabled();
> > + }
> > +
> > if (is_broadcast_ether_addr(ehdr->h_dest)) {
> > for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> > if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) { @@
> > -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore *core,
> const struct eth_header *ehdr,
> > }
> > }
> >
> > + /* assume a full pool list if IGMAC is set */
> > + if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
> > + queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
> > + }
> > +
>
> This overwrites "queues", but "external_tx" is not overwritten.
Description in section 7.10.3.6 is a bit confusing, I interpreted that packet is not sent to network if it matches an exact filter regardless of VT_CTL.IGMAC setting.
I think that VT_CTL.IGMAC setting is only for MAC filtering and pool selection, and not to determine if a packet must go to external LAN or not.
>
> > if (e1000x_vlan_rx_filter_enabled(core->mac)) {
> > uint16_t mask = 0;
> >
> > diff --git a/hw/net/trace-events b/hw/net/trace-events index
> > e94172e748..9bc7658692 100644
> > --- a/hw/net/trace-events
> > +++ b/hw/net/trace-events
> > @@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t
> > offset, const void* source, uint3
> >
> > igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
> >
> > +igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication
> mode enabled is supported"
> > +
> > igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR
> enabled"
> > igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr)
> "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
> > igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
2023-02-01 10:29 ` Sriram Yagnaraman
@ 2023-02-01 13:03 ` Akihiko Odaki
2023-02-02 7:24 ` Sriram Yagnaraman
0 siblings, 1 reply; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 13:03 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel@nongnu.org, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum
On 2023/02/01 19:29, Sriram Yagnaraman wrote:
>
>
>> -----Original Message-----
>> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>> Sent: Wednesday, 1 February 2023 05:58
>> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
>> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
>> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
>> Subject: Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
>>
>> On 2023/01/31 18:42, Sriram Yagnaraman wrote:
>>> Also trace out a warning if replication mode is disabled, since we
>>> only support replication mode enabled.
>>>
>>> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
>>> ---
>>> hw/net/igb_core.c | 9 +++++++++
>>> hw/net/trace-events | 2 ++
>>> 2 files changed, 11 insertions(+)
>>>
>>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
>>> c5f9c14f47..8115be2d76 100644
>>> --- a/hw/net/igb_core.c
>>> +++ b/hw/net/igb_core.c
>>> @@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore *core,
>> const struct eth_header *ehdr,
>>> }
>>>
>>> if (core->mac[MRQC] & 1) {
>>> + if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
>>> + trace_igb_rx_vmdq_replication_mode_disabled();
>>> + }
>>> +
>>> if (is_broadcast_ether_addr(ehdr->h_dest)) {
>>> for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
>>> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) { @@
>>> -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore *core,
>> const struct eth_header *ehdr,
>>> }
>>> }
>>>
>>> + /* assume a full pool list if IGMAC is set */
>>> + if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
>>> + queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
>>> + }
>>> +
>>
>> This overwrites "queues", but "external_tx" is not overwritten.
>
> Description in section 7.10.3.6 is a bit confusing, I interpreted that packet is not sent to network if it matches an exact filter regardless of VT_CTL.IGMAC setting.
> I think that VT_CTL.IGMAC setting is only for MAC filtering and pool selection, and not to determine if a packet must go to external LAN or not.
It says nothing about VT_CTL.IGMAC so we need to make the best guess.
The rule saying "a unicast packet that matches an exact filter is not
sent to the LAN" aligns with the common expectation of driver authors
that a packet directed to a VF won't be sent to someone else.
However, when VT_CTL.IGMAC is set, the exact filter does not tell if the
destination of the packet is a VF. In such a case, that rule does not do
anything good, but can do some harm; if you have used igb for normal MAC
routing and later switched to VLAN routing with VT_CTL.IGMAC, the exact
filter may be left as is, the exact filter can prevent irrelevant
packets from being routed to the external LAN.
>
>>
>>> if (e1000x_vlan_rx_filter_enabled(core->mac)) {
>>> uint16_t mask = 0;
>>>
>>> diff --git a/hw/net/trace-events b/hw/net/trace-events index
>>> e94172e748..9bc7658692 100644
>>> --- a/hw/net/trace-events
>>> +++ b/hw/net/trace-events
>>> @@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t
>>> offset, const void* source, uint3
>>>
>>> igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
>>>
>>> +igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication
>> mode enabled is supported"
>>> +
>>> igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to GPIE.NSICR
>> enabled"
>>> igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t new_icr)
>> "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
>>> igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
^ permalink raw reply [flat|nested] 19+ messages in thread* RE: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
2023-02-01 13:03 ` Akihiko Odaki
@ 2023-02-02 7:24 ` Sriram Yagnaraman
0 siblings, 0 replies; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-02-02 7:24 UTC (permalink / raw)
To: Akihiko Odaki
Cc: qemu-devel@nongnu.org, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum
> -----Original Message-----
> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> Sent: Wednesday, 1 February 2023 14:03
> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>; Dmitry
> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Subject: Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
>
> On 2023/02/01 19:29, Sriram Yagnaraman wrote:
> >
> >
> >> -----Original Message-----
> >> From: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> Sent: Wednesday, 1 February 2023 05:58
> >> To: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> >> Cc: qemu-devel@nongnu.org; Jason Wang <jasowang@redhat.com>;
> Dmitry
> >> Fleytman <dmitry.fleytman@gmail.com>; Michael S . Tsirkin
> >> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> >> Subject: Re: [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field
> >>
> >> On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> >>> Also trace out a warning if replication mode is disabled, since we
> >>> only support replication mode enabled.
> >>>
> >>> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> >>> ---
> >>> hw/net/igb_core.c | 9 +++++++++
> >>> hw/net/trace-events | 2 ++
> >>> 2 files changed, 11 insertions(+)
> >>>
> >>> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c index
> >>> c5f9c14f47..8115be2d76 100644
> >>> --- a/hw/net/igb_core.c
> >>> +++ b/hw/net/igb_core.c
> >>> @@ -964,6 +964,10 @@ static uint16_t igb_receive_assign(IGBCore
> >>> *core,
> >> const struct eth_header *ehdr,
> >>> }
> >>>
> >>> if (core->mac[MRQC] & 1) {
> >>> + if (!(core->mac[VT_CTL] & E1000_VT_CTL_VM_REPL_EN)) {
> >>> + trace_igb_rx_vmdq_replication_mode_disabled();
> >>> + }
> >>> +
> >>> if (is_broadcast_ether_addr(ehdr->h_dest)) {
> >>> for (i = 0; i < IGB_NUM_VM_POOLS; i++) {
> >>> if (core->mac[VMOLR0 + i] & E1000_VMOLR_BAM) { @@
> >>> -1010,6 +1014,11 @@ static uint16_t igb_receive_assign(IGBCore
> >>> *core,
> >> const struct eth_header *ehdr,
> >>> }
> >>> }
> >>>
> >>> + /* assume a full pool list if IGMAC is set */
> >>> + if (core->mac[VT_CTL] & E1000_VT_CTL_IGNORE_MAC) {
> >>> + queues = BIT(IGB_MAX_VF_FUNCTIONS) - 1;
> >>> + }
> >>> +
> >>
> >> This overwrites "queues", but "external_tx" is not overwritten.
> >
> > Description in section 7.10.3.6 is a bit confusing, I interpreted that packet is
> not sent to network if it matches an exact filter regardless of VT_CTL.IGMAC
> setting.
> > I think that VT_CTL.IGMAC setting is only for MAC filtering and pool
> selection, and not to determine if a packet must go to external LAN or not.
>
> It says nothing about VT_CTL.IGMAC so we need to make the best guess.
>
> The rule saying "a unicast packet that matches an exact filter is not sent to the
> LAN" aligns with the common expectation of driver authors that a packet
> directed to a VF won't be sent to someone else.
>
> However, when VT_CTL.IGMAC is set, the exact filter does not tell if the
> destination of the packet is a VF. In such a case, that rule does not do anything
> good, but can do some harm; if you have used igb for normal MAC routing and
> later switched to VLAN routing with VT_CTL.IGMAC, the exact filter may be
> left as is, the exact filter can prevent irrelevant packets from being routed to
> the external LAN.
Okay, that makes sense. Anyhow, I think it is better to implement DTXSWC.LLE bit to keep/remove source pool before implementing this change. Otherwise, we might end up sending packets back to the originating VF when VLAN filtering allows it.
>
> >
> >>
> >>> if (e1000x_vlan_rx_filter_enabled(core->mac)) {
> >>> uint16_t mask = 0;
> >>>
> >>> diff --git a/hw/net/trace-events b/hw/net/trace-events index
> >>> e94172e748..9bc7658692 100644
> >>> --- a/hw/net/trace-events
> >>> +++ b/hw/net/trace-events
> >>> @@ -288,6 +288,8 @@ igb_rx_desc_buff_write(uint64_t addr, uint16_t
> >>> offset, const void* source, uint3
> >>>
> >>> igb_rx_metadata_rss(uint32_t rss) "RSS data: 0x%X"
> >>>
> >>> +igb_rx_vmdq_replication_mode_disabled(void) "WARN: Only replication
> >> mode enabled is supported"
> >>> +
> >>> igb_irq_icr_clear_gpie_nsicr(void) "Clearing ICR on read due to
> >>> GPIE.NSICR
> >> enabled"
> >>> igb_irq_icr_write(uint32_t bits, uint32_t old_icr, uint32_t
> >>> new_icr)
> >> "Clearing ICR bits 0x%x: 0x%x --> 0x%x"
> >>> igb_irq_set_iam(uint32_t icr) "Update IAM: 0x%x"
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v3 9/9] igb: respect VMVIR and VMOLR for VLAN
2023-01-31 9:42 [PATCH v3 0/9] igb: merge changes from <20221229190817.25500-1-sriram.yagnaraman@est.tech> Sriram Yagnaraman
` (7 preceding siblings ...)
2023-01-31 9:42 ` [PATCH v3 8/9] igb: respect VT_CTL ignore MAC field Sriram Yagnaraman
@ 2023-01-31 9:42 ` Sriram Yagnaraman
2023-02-01 4:53 ` Akihiko Odaki
8 siblings, 1 reply; 19+ messages in thread
From: Sriram Yagnaraman @ 2023-01-31 9:42 UTC (permalink / raw)
Cc: qemu-devel, Akihiko Odaki, Jason Wang, Dmitry Fleytman,
Michael S . Tsirkin, Marcel Apfelbaum, Sriram Yagnaraman
Add support for stripping/inserting VLAN for VFs.
Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
---
hw/net/igb_core.c | 51 ++++++++++++++++++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 9 deletions(-)
diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
index 8115be2d76..a697fcf56a 100644
--- a/hw/net/igb_core.c
+++ b/hw/net/igb_core.c
@@ -386,6 +386,25 @@ igb_rss_parse_packet(IGBCore *core, struct NetRxPkt *pkt, bool tx,
info->queue = E1000_RSS_QUEUE(&core->mac[RETA], info->hash);
}
+static inline bool
+igb_tx_insert_vlan(IGBCore *core, uint16_t qn,
+ struct igb_tx *tx, bool desc_vle)
+{
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = qn % IGB_NUM_VM_POOLS;
+
+ if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_DEFAULT) {
+ /* always insert default VLAN */
+ desc_vle = true;
+ tx->vlan = core->mac[VMVIR0 + pool] & 0xffff;
+ } else if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_NEVER) {
+ return false;
+ }
+ }
+
+ return desc_vle && e1000x_vlan_enabled(core->mac);
+}
+
static bool
igb_setup_tx_offloads(IGBCore *core, struct igb_tx *tx)
{
@@ -581,7 +600,8 @@ igb_process_tx_desc(IGBCore *core,
if (cmd_type_len & E1000_TXD_CMD_EOP) {
if (!tx->skip_cp && net_tx_pkt_parse(tx->tx_pkt)) {
- if (cmd_type_len & E1000_TXD_CMD_VLE) {
+ if (igb_tx_insert_vlan(core, queue_index, tx,
+ !!(cmd_type_len & E1000_TXD_CMD_VLE))) {
net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt, tx->vlan,
core->mac[VET] & 0xffff);
}
@@ -1543,6 +1563,20 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
igb_update_rx_stats(core, rxi, size, total_size);
}
+static bool
+igb_rx_strip_vlan(IGBCore *core, const E1000E_RingInfo *rxi)
+{
+ if (core->mac[MRQC] & 1) {
+ uint16_t pool = rxi->idx % IGB_NUM_VM_POOLS;
+ /* Sec 7.10.3.8: CTRL.VME is ignored, only VMOLR/RPLOLR is used */
+ return (net_rx_pkt_get_packet_type(core->rx_pkt) == ETH_PKT_MCAST) ?
+ core->mac[RPLOLR] & E1000_RPLOLR_STRVLAN :
+ core->mac[VMOLR0 + pool] & E1000_VMOLR_STRVLAN;
+ }
+
+ return e1000x_vlan_enabled(core->mac);
+}
+
static inline void
igb_rx_fix_l4_csum(IGBCore *core, struct NetRxPkt *pkt)
{
@@ -1624,10 +1658,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
ehdr = PKT_GET_ETH_HDR(filter_buf);
net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
-
- net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
- e1000x_vlan_enabled(core->mac),
- core->mac[VET] & 0xffff);
+ net_rx_pkt_set_protocols(core->rx_pkt, filter_buf, size);
queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
if (!queues) {
@@ -1635,11 +1666,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
return orig_size;
}
- total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
- e1000x_fcs_len(core->mac);
-
retval = orig_size;
- igb_rx_fix_l4_csum(core, core->rx_pkt);
+ total_size = size + e1000x_fcs_len(core->mac);
for (i = 0; i < IGB_NUM_QUEUES; i++) {
if (!(queues & BIT(i)) ||
@@ -1648,6 +1676,11 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
}
igb_rx_ring_init(core, &rxr, i);
+ net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
+ igb_rx_strip_vlan(core, rxr.i),
+ core->mac[VET] & 0xffff);
+ igb_rx_fix_l4_csum(core, core->rx_pkt);
+
if (!igb_has_rxbufs(core, rxr.i, total_size)) {
icr_bits |= E1000_ICS_RXO;
continue;
--
2.34.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v3 9/9] igb: respect VMVIR and VMOLR for VLAN
2023-01-31 9:42 ` [PATCH v3 9/9] igb: respect VMVIR and VMOLR for VLAN Sriram Yagnaraman
@ 2023-02-01 4:53 ` Akihiko Odaki
0 siblings, 0 replies; 19+ messages in thread
From: Akihiko Odaki @ 2023-02-01 4:53 UTC (permalink / raw)
To: Sriram Yagnaraman
Cc: qemu-devel, Jason Wang, Dmitry Fleytman, Michael S . Tsirkin,
Marcel Apfelbaum
On 2023/01/31 18:42, Sriram Yagnaraman wrote:
> Add support for stripping/inserting VLAN for VFs.
>
> Signed-off-by: Sriram Yagnaraman <sriram.yagnaraman@est.tech>
> ---
> hw/net/igb_core.c | 51 ++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 42 insertions(+), 9 deletions(-)
>
> diff --git a/hw/net/igb_core.c b/hw/net/igb_core.c
> index 8115be2d76..a697fcf56a 100644
> --- a/hw/net/igb_core.c
> +++ b/hw/net/igb_core.c
> @@ -386,6 +386,25 @@ igb_rss_parse_packet(IGBCore *core, struct NetRxPkt *pkt, bool tx,
> info->queue = E1000_RSS_QUEUE(&core->mac[RETA], info->hash);
> }
>
> +static inline bool
> +igb_tx_insert_vlan(IGBCore *core, uint16_t qn,
> + struct igb_tx *tx, bool desc_vle)
> +{
> + if (core->mac[MRQC] & 1) {
> + uint16_t pool = qn % IGB_NUM_VM_POOLS;
> +
> + if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_DEFAULT) {
> + /* always insert default VLAN */
> + desc_vle = true;
> + tx->vlan = core->mac[VMVIR0 + pool] & 0xffff;
> + } else if (core->mac[VMVIR0 + pool] & E1000_VMVIR_VLANA_NEVER) {
> + return false;
> + }
> + }
> +
> + return desc_vle && e1000x_vlan_enabled(core->mac);
> +}
> +
> static bool
> igb_setup_tx_offloads(IGBCore *core, struct igb_tx *tx)
> {
> @@ -581,7 +600,8 @@ igb_process_tx_desc(IGBCore *core,
>
> if (cmd_type_len & E1000_TXD_CMD_EOP) {
> if (!tx->skip_cp && net_tx_pkt_parse(tx->tx_pkt)) {
> - if (cmd_type_len & E1000_TXD_CMD_VLE) {
> + if (igb_tx_insert_vlan(core, queue_index, tx,
> + !!(cmd_type_len & E1000_TXD_CMD_VLE))) {
> net_tx_pkt_setup_vlan_header_ex(tx->tx_pkt, tx->vlan,
> core->mac[VET] & 0xffff);
> }
> @@ -1543,6 +1563,20 @@ igb_write_packet_to_guest(IGBCore *core, struct NetRxPkt *pkt,
> igb_update_rx_stats(core, rxi, size, total_size);
> }
>
> +static bool
> +igb_rx_strip_vlan(IGBCore *core, const E1000E_RingInfo *rxi)
> +{
> + if (core->mac[MRQC] & 1) {
> + uint16_t pool = rxi->idx % IGB_NUM_VM_POOLS;
> + /* Sec 7.10.3.8: CTRL.VME is ignored, only VMOLR/RPLOLR is used */
> + return (net_rx_pkt_get_packet_type(core->rx_pkt) == ETH_PKT_MCAST) ?
> + core->mac[RPLOLR] & E1000_RPLOLR_STRVLAN :
> + core->mac[VMOLR0 + pool] & E1000_VMOLR_STRVLAN;
> + }
> +
> + return e1000x_vlan_enabled(core->mac);
> +}
> +
> static inline void
> igb_rx_fix_l4_csum(IGBCore *core, struct NetRxPkt *pkt)
> {
> @@ -1624,10 +1658,7 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
>
> ehdr = PKT_GET_ETH_HDR(filter_buf);
> net_rx_pkt_set_packet_type(core->rx_pkt, get_eth_packet_type(ehdr));
> -
> - net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
> - e1000x_vlan_enabled(core->mac),
> - core->mac[VET] & 0xffff);
> + net_rx_pkt_set_protocols(core->rx_pkt, filter_buf, size);
>
> queues = igb_receive_assign(core, ehdr, size, &rss_info, external_tx);
> if (!queues) {
> @@ -1635,11 +1666,8 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> return orig_size;
> }
>
> - total_size = net_rx_pkt_get_total_len(core->rx_pkt) +
> - e1000x_fcs_len(core->mac);
> -
> retval = orig_size;
> - igb_rx_fix_l4_csum(core, core->rx_pkt);
> + total_size = size + e1000x_fcs_len(core->mac);
This change to total_size should be reverted; total_size will be
different from size if VLAN stripping is enabled. There is also no
reason to reorder the statements.
>
> for (i = 0; i < IGB_NUM_QUEUES; i++) {
> if (!(queues & BIT(i)) ||
> @@ -1648,6 +1676,11 @@ igb_receive_internal(IGBCore *core, const struct iovec *iov, int iovcnt,
> }
>
> igb_rx_ring_init(core, &rxr, i);
> + net_rx_pkt_attach_iovec_ex(core->rx_pkt, iov, iovcnt, iov_ofs,
> + igb_rx_strip_vlan(core, rxr.i),
> + core->mac[VET] & 0xffff);
> + igb_rx_fix_l4_csum(core, core->rx_pkt);
> +
> if (!igb_has_rxbufs(core, rxr.i, total_size)) {
> icr_bits |= E1000_ICS_RXO;
> continue;
^ permalink raw reply [flat|nested] 19+ messages in thread