On 08/06/2022 10:09, Kuo-Hsiang Chou wrote:
> Hi Thomas
>
> Thanks for your suggestions!
>
> I answer each revision inline that followed by [KH]:.
Thanks for reviewing this.
>
> Regards,
>
> Kuo-Hsiang Chou
>
> -----Original Message-----
>
> From: Thomas Zimmermann [mailto:tzimmermann@suse.de]
>
> Sent: Tuesday, June 07, 2022 8:03 PM
>
> To: airlied(a)redhat.com; airlied(a)linux.ie; daniel(a)ffwll.ch;
> jfalempe(a)redhat.com; regressions(a)leemhuis.info; Kuo-Hsiang Chou
> <kuohsiang_chou(a)aspeedtech.com>
>
> Subject: [PATCH] drm/ast: Treat AST2600 like AST2500 in most places
>
> Include AST2600 in most of the branches for AST2500. Thereby revert most
> effects of commit f9bd00e0ea9d ("drm/ast: Create chip AST2600").
>
> The AST2600 used to be treated like an AST2500, which at least gave
> usable display output. After introducing AST2600 in the driver without
> further updates, lots of functions take the wrong branches.
>
> Handling AST2600 in the AST2500 branches reverts back to the original
> settings. The exception are cases where AST2600 meanwhile got its own
> branch.
>
> [KH]: Based on CVE_2019_6260 item3, P2A is disallowed anymore.
>
> P2A (PCIe to AMBA) is a bridge that is able to revise any BMC registers.
>
> Yes, P2A is dangerous on security issue, because Host open a backdoor
> and someone malicious SW/APP will be easy to take control of BMC.
>
> Therefore, P2A is disabled forever.
>
> Now, return to this patch, there is no need to add AST2600 condition on
> the P2A flow.
>
[snip]
>
> [KH]: Yes, the patch is "drm/ast: Create threshold values for AST2600"
> that is the root cause of whites lines on AST2600
>
> commit
>
>
> bcc77411e8a65929655cef7b63a36000724cdc4b
> <https://cgit.freedesktop.org/drm/drm/commit/?id=bcc77411e8a65929655cef7b63a…> (patch
> <https://cgit.freedesktop.org/drm/drm/patch/?id=bcc77411e8a65929655cef7b63a3…>)
>
So basically this commit should be enough to fix the white lines and
flickering with VGA output on AST2600 ?
I will try to have it tested, and if it's good, we may want to have it
on stable kernel.
Best regards,
--
Jocelyn
The revision of the imx-sdma IP that is in the i.MX8M series is the
same is that as that in the i.MX7 series but the imx7d MODULE_FIRMWARE
directive is wrapped in a condiditional which means it's not defined
when built for aarch64 SOC_IMX8M platforms and hence you get the
following errors when the driver loads on imx8m devices:
imx-sdma 302c0000.dma-controller: Direct firmware load for imx/sdma/sdma-imx7d.bin failed with error -2
imx-sdma 302c0000.dma-controller: external firmware not found, using ROM firmware
Add the SOC_IMX8M into the check so the firmware can load on i.MX8.
Fixes: 1474d48bd639 ("arm64: dts: imx8mq: Add SDMA nodes")
Fixes: 941acd566b18 ("dmaengine: imx-sdma: Only check ratio on parts that support 1:1")
Signed-off-by: Peter Robinson <pbrobinson(a)gmail.com>
Cc: stable(a)vger.kernel.org # v5.2+
---
drivers/dma/imx-sdma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 8535018ee7a2..900cafdaf359 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2346,7 +2346,7 @@ MODULE_DESCRIPTION("i.MX SDMA driver");
#if IS_ENABLED(CONFIG_SOC_IMX6Q)
MODULE_FIRMWARE("imx/sdma/sdma-imx6q.bin");
#endif
-#if IS_ENABLED(CONFIG_SOC_IMX7D)
+#if IS_ENABLED(CONFIG_SOC_IMX7D) || IS_ENABLED(CONFIG_SOC_IMX8M)
MODULE_FIRMWARE("imx/sdma/sdma-imx7d.bin");
#endif
MODULE_LICENSE("GPL");
--
2.36.1
commit b9684a71fca793213378dd410cd11675d973eaa1 upstream.
Historically we did distinguish between a flag that surpressed partition
scanning, and a combinations of the minors variable and another flag if
any partitions were supported. This was generally confusing and doesn't
make much sense, but some corner case uses of the loop driver actually
do want to support manually added partitions on a device that does not
actively scan for partitions. To make things worsee the loop driver
also wants to dynamically toggle the scanning for partitions on a live
gendisk, which makes the disk->flags updates non-atomic.
Introduce a new GD_SUPPRESS_PART_SCAN bit in disk->state that disables
just scanning for partitions, and toggle that instead of GENHD_FL_NO_PART
in the loop driver.
Fixes: 1ebe2e5f9d68 ("block: remove GENHD_FL_EXT_DEVT")
Reported-by: Ming Lei <ming.lei(a)redhat.com>
Signed-off-by: Christoph Hellwig <hch(a)lst.de>
Reviewed-by: Ming Lei <ming.lei(a)redhat.com>
Link: https://lore.kernel.org/r/20220527055806.1972352-1-hch@lst.de
Signed-off-by: Jens Axboe <axboe(a)kernel.dk>
---
block/genhd.c | 2 ++
drivers/block/loop.c | 8 ++++----
include/linux/blkdev.h | 1 +
3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/genhd.c b/block/genhd.c
index b8b6759d670f01..3008ec2136543e 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -385,6 +385,8 @@ int disk_scan_partitions(struct gendisk *disk, fmode_t mode)
if (disk->flags & (GENHD_FL_NO_PART | GENHD_FL_HIDDEN))
return -EINVAL;
+ if (test_bit(GD_SUPPRESS_PART_SCAN, &disk->state))
+ return -EINVAL;
if (disk->open_partitions)
return -EBUSY;
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index a58595f5ee2c8f..ba3627f982f241 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1066,7 +1066,7 @@ static int loop_configure(struct loop_device *lo, fmode_t mode,
lo->lo_flags |= LO_FLAGS_PARTSCAN;
partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
if (partscan)
- lo->lo_disk->flags &= ~GENHD_FL_NO_PART;
+ clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
loop_global_unlock(lo, is_loop);
if (partscan)
@@ -1185,7 +1185,7 @@ static void __loop_clr_fd(struct loop_device *lo, bool release)
*/
lo->lo_flags = 0;
if (!part_shift)
- lo->lo_disk->flags |= GENHD_FL_NO_PART;
+ set_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
mutex_lock(&lo->lo_mutex);
lo->lo_state = Lo_unbound;
mutex_unlock(&lo->lo_mutex);
@@ -1295,7 +1295,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
if (!err && (lo->lo_flags & LO_FLAGS_PARTSCAN) &&
!(prev_lo_flags & LO_FLAGS_PARTSCAN)) {
- lo->lo_disk->flags &= ~GENHD_FL_NO_PART;
+ clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
partscan = true;
}
out_unlock:
@@ -2045,7 +2045,7 @@ static int loop_add(int i)
* userspace tools. Parameters like this in general should be avoided.
*/
if (!part_shift)
- disk->flags |= GENHD_FL_NO_PART;
+ set_bit(GD_SUPPRESS_PART_SCAN, &disk->state);
atomic_set(&lo->lo_refcnt, 0);
mutex_init(&lo->lo_mutex);
lo->lo_number = i;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 60d01613899711..108e3d114bfc1f 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -147,6 +147,7 @@ struct gendisk {
#define GD_DEAD 2
#define GD_NATIVE_CAPACITY 3
#define GD_ADDED 4
+#define GD_SUPPRESS_PART_SCAN 5
struct mutex open_mutex; /* open/close mutex */
unsigned open_partitions; /* number of open partitions */
--
2.30.2
If packet headers changed, the cached nfct is no longer relevant
for the packet and attempt to re-use it leads to the incorrect packet
classification.
This issue is causing broken connectivity in OpenStack deployments
with OVS/OVN due to hairpin traffic being unexpectedly dropped.
The setup has datapath flows with several conntrack actions and tuple
changes between them:
actions:ct(commit,zone=8,mark=0/0x1,nat(src)),
set(eth(src=00:00:00:00:00:01,dst=00:00:00:00:00:06)),
set(ipv4(src=172.18.2.10,dst=192.168.100.6,ttl=62)),
ct(zone=8),recirc(0x4)
After the first ct() action the packet headers are almost fully
re-written. The next ct() tries to re-use the existing nfct entry
and marks the packet as invalid, so it gets dropped later in the
pipeline.
Clearing the cached conntrack entry whenever packet tuple is changed
to avoid the issue.
The flow key should not be cleared though, because we should still
be able to match on the ct_state if the recirculation happens after
the tuple change but before the next ct() action.
Cc: stable(a)vger.kernel.org
Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Reported-by: Frode Nordahl <frode.nordahl(a)canonical.com>
Link: https://mail.openvswitch.org/pipermail/ovs-discuss/2022-May/051829.html
Link: https://bugs.launchpad.net/ubuntu/+source/ovn/+bug/1967856
Signed-off-by: Ilya Maximets <i.maximets(a)ovn.org>
---
The function ovs_ct_clear() looks a bit differently on older branches,
but the change should be exactly the same, i.e. move the
ovs_ct_fill_key() under the 'if (key)'.
The same behavior for userspace datapath was introduced along with
the conntrack caching support here:
https://github.com/openvswitch/ovs/commit/594570ea1cdecc7ef7880d707cbc7a4a4…
Interestingly, above commit also introduced the system test that can
check the issue for the kernel as well, but the test sends only one
packet and this packet goes via upcall to userspace and back to the
kernel effectively clearing the cached connection along the way and
avoiding the issue. If the test is modified to send more than a few
packets [1], it starts to fail without the kernel fix:
make check-kernel TESTSUITEFLAGS='-k negative'
142: conntrack - negative test for recirculation optimization FAILED
[1] https://pastebin.com/H1YMqaLa
net/openvswitch/actions.c | 6 ++++++
net/openvswitch/conntrack.c | 4 +++-
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 1b5d73079dc9..868db4669a29 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -373,6 +373,7 @@ static void set_ip_addr(struct sk_buff *skb, struct iphdr *nh,
update_ip_l4_checksum(skb, nh, *addr, new_addr);
csum_replace4(&nh->check, *addr, new_addr);
skb_clear_hash(skb);
+ ovs_ct_clear(skb, NULL);
*addr = new_addr;
}
@@ -420,6 +421,7 @@ static void set_ipv6_addr(struct sk_buff *skb, u8 l4_proto,
update_ipv6_checksum(skb, l4_proto, addr, new_addr);
skb_clear_hash(skb);
+ ovs_ct_clear(skb, NULL);
memcpy(addr, new_addr, sizeof(__be32[4]));
}
@@ -660,6 +662,7 @@ static int set_nsh(struct sk_buff *skb, struct sw_flow_key *flow_key,
static void set_tp_port(struct sk_buff *skb, __be16 *port,
__be16 new_port, __sum16 *check)
{
+ ovs_ct_clear(skb, NULL);
inet_proto_csum_replace2(check, skb, *port, new_port, false);
*port = new_port;
}
@@ -699,6 +702,7 @@ static int set_udp(struct sk_buff *skb, struct sw_flow_key *flow_key,
uh->dest = dst;
flow_key->tp.src = src;
flow_key->tp.dst = dst;
+ ovs_ct_clear(skb, NULL);
}
skb_clear_hash(skb);
@@ -761,6 +765,8 @@ static int set_sctp(struct sk_buff *skb, struct sw_flow_key *flow_key,
sh->checksum = old_csum ^ old_correct_csum ^ new_csum;
skb_clear_hash(skb);
+ ovs_ct_clear(skb, NULL);
+
flow_key->tp.src = sh->source;
flow_key->tp.dst = sh->dest;
diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 4a947c13c813..4e70df91d0f2 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -1342,7 +1342,9 @@ int ovs_ct_clear(struct sk_buff *skb, struct sw_flow_key *key)
nf_ct_put(ct);
nf_ct_set(skb, NULL, IP_CT_UNTRACKED);
- ovs_ct_fill_key(skb, key, false);
+
+ if (key)
+ ovs_ct_fill_key(skb, key, false);
return 0;
}
--
2.34.3