The sender domain has a DMARC Reject/Quarantine policy which disallows
sending mailing list messages using the original "From" header.

To mitigate this problem, the original message has been wrapped
automatically by the mailing list software.
--- Begin Message ---
Hello.

I sent the first patch (support for differently named partitions) now.

Will follow up with the other patches as soon as we agree on this one. As 
stated in the commit message, I believe the "secondary" partitions (which are 
actually named xxxx@0, while the primaries are "@1") are populated by the OEM 
firmware. I may be mistaken, but at least I can confirm that with a non-working 
software upgrade, the next boot resulted in the device reverting to the 
previous firmware version I had, which was OEM.

-And auto-split probably  works, I just hadn't read through that part of the 
code, and seen that I needed to handle the specific partition names there as 
well. I am testing this now.


Regards,

Ole Kristian

On 24/04/2022, 10:28, "Sander Vanheule" <san...@svanheule.net> wrote:

    Hi Ole Kristian,


    On Tue, 2022-04-19 at 15:46 +0200, Ole Kristian Lona wrote:
    > 
    > Support for creating images for TP-Link Deco M4R version 3, and for M5.

    Please provide separate patches for:
     * image generation code changes
     * Deco M4R v3
     * Deco M5 

    > 
    > Partition table and supportlists were extracted from newest OEM image.
    > 
    > Both devices have partition tables with fallback partitions, like:
    > os-image@0 and os-image@1, file-system@0 and file-system@1
    > 
    > This is assumed to be a fail-safe built into the firmware of the 
    > devices.

    These devices are not explicitly dual boot then? How does the bootloader 
decide which os-
    image will be loaded? If the device alternates between @0 and @1 on 
firmware upgrades, you
    will need to specify in the factory install instructions how to select the 
correct image
    set.

    > Therefore, this naming scheme has been kept, and tplink-safeloader.c
    > has been slightly modified to account for this.
    > 
    > I have tested without these "@1" and "@0" names. This causes the 
    > firmware to be
    > makred as invalid. Therefore, I made changes to the logic in the app, 
    > (re?)adding a name
    > parameter to functions, and (last part of the patch) making sure these 
    > names are
    > sent to the functions correctly.
    > 
    > Also tested merging os-image and file-system into one "firmware" 
    > partition. This rendered
    > the firmware invalid, so reverted back to original TP-Link layout.

    Is this true when "firmware" is auto-split into "os-image@1" and 
"file-system@1" too?
    Typically, when you keep the fixed split, an update will be required in the 
future because
    the kernel has outgrown the small (4MB in this case) os-image partition.

    > 
    > Signed-off-by: Ole Kristian Lona <o...@oklona.net>
    > ---
    >   src/tplink-safeloader.c | 169 +++++++++++++++++++++++++++++++++++++---
    >   1 file changed, 159 insertions(+), 10 deletions(-)
    > 
    > diff --git a/src/tplink-safeloader.c b/src/tplink-safeloader.c
    > index c4727df..7b5d2d4 100644
    > --- a/src/tplink-safeloader.c
    > +++ b/src/tplink-safeloader.c
    > 

    ...

    > @@ -2895,8 +3026,8 @@ static void set_source_date_epoch() {
    >   }
    > 
    >   /** Generates the partition-table partition */
    > -static struct image_partition_entry make_partition_table(const struct 
    > flash_partition_entry *p) {
    > -       struct image_partition_entry entry = 
    > alloc_image_partition("partition-table", 0x800);
    > +static struct image_partition_entry make_partition_table(const char 
    > *name,const struct flash_partition_entry *p) {

    Add a space after the comma

    > +       struct image_partition_entry entry = alloc_image_partition(name, 
    > 0x800);
    > 
    >         char *s = (char *)entry.data, *end = (char *)(s+entry.size);

    ...

    > 
    > @@ -3213,6 +3344,11 @@ static void build_image(const char *output,
    > 
    >         size_t i;
    > 
    > +       char *part_table_name;
    > +       char *soft_ver_name;
    > +       char *os_im_name;
    > +       char *fs_name;
    > +
    >         struct image_partition_entry parts[7] = {};
    > 
    >         struct flash_partition_entry *firmware_partition = NULL;
    > @@ -3256,11 +3392,24 @@ static void build_image(const char *output,
    >                 os_image_partition->size = kernel.st_size;
    >         }
    > 
    > -       parts[0] = make_partition_table(info->partitions);
    > -       parts[1] = make_soft_version(info, rev);
    > +       if (strcasecmp(info->id, "DECO-M4R-V3") == 0 ||
    > +               strcasecmp(info->id, "DECO-M5") == 0 ) {
    > +               part_table_name="partition-table@1";
    > +               soft_ver_name="soft-version@1";
    > +               os_im_name="os-image@1";
    > +               fs_name="file-system@1";
    > +       } else {
    > +               part_table_name="partition-table";
    > +               soft_ver_name="soft-version";
    > +               os_im_name="os-image";
    > +               fs_name="file-system";
    > +       }
    > +
    > +       parts[0] = make_partition_table(part_table_name,info->partitions);
    > +       parts[1] = make_soft_version(soft_ver_name,info, rev);
    >         parts[2] = make_support_list(info);
    > -       parts[3] = read_file("os-image", kernel_image, false, NULL);
    > -       parts[4] = read_file("file-system", rootfs_image, add_jffs2_eof, 
    > file_system_partition);
    > +       parts[3] = read_file(os_im_name, kernel_image, false, NULL);
    > +       parts[4] = read_file(fs_name, rootfs_image, add_jffs2_eof, 
    > file_system_partition);
    > 
    >         /* Some devices need the extra-para partition to accept the 
firmware 
    > */
    >         if (strcasecmp(info->id, "ARCHER-A6-V3") == 0 ||

    I'm aware a strcasecmp() is performed for extra-para partition, but I don't 
particularly
    like that either. This puts device-specific details in different places, 
and after a while
    you don't know where to look anymore to add support for a new device.

    The list of generated partition for factory images is fixed. You could 
create a new struct
    (e.g. 'factory_partition_names') that contains names for all required 
partitions:

    struct factory_partition_names {
    const char *partition_table;
    const char *soft_ver;
    const char *support_list;
    const char *os_image;
    const char *file_system;
    const char *extra_para;
    };

    Then, add a member to the device_info struct that points to a 
factory_partition_names
    object. For example:

    static const struct factory_partition_names PARTITION_NAMES_DUALBOOT_DECO_M 
= {
    "partition-table@1",
    "soft-version@1",
    "support-list",
    "os-image@1",
    "file-system@1",
    NULL /* extra-para is not used on these devices */
    };

    If the partition names pointer is NULL (e.g. when left unspecified), then 
you can default
    to current set of names. If the pointer is not NULL, take the names from 
the pointed-to
    struct. Then all you need to pass around is the device_info pointer, and 
the partition
    generator functions can grab all the required info from there.

    Also bear in mind that the partition names are not only used to generate 
OpenWrt factory
    images, but also to convert OEM firmware images to sysupgrade-compatible 
files. But
    solving that problem can be a separate patch, so let's do factory images 
first.


    Best,
    Sander




--- End Message ---
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel

Reply via email to