| Message ID | 20210831034536.17497-1-xiaoliang.yang_1@nxp.com |
|---|---|
| Headers | show |
| Series | net: dsa: felix: psfp support on vsc9959 | expand |
On Tue, Aug 31, 2021 at 11:45:33AM +0800, Xiaoliang Yang wrote: > +static int vsc9959_mact_stream_set(struct ocelot *ocelot, > + struct felix_stream *stream, > + struct netlink_ext_ack *extack) > +{ > + struct ocelot_mact_entry entry; > + u32 row, col, reg, dst_idx; > + int ret; > + > + /* Stream identification desn't support to add a stream with non > + * existent MAC (The MAC entry has not been learned in MAC table). > + */ Who will add the MAC entry to the MAC table in this design? The user? > + ret = ocelot_mact_lookup(ocelot, stream->dmac, stream->vid, &row, &col); > + if (ret) { > + if (extack) > + NL_SET_ERR_MSG_MOD(extack, "Stream is not learned in MAC table"); > + return -EOPNOTSUPP; > + } > + > + ocelot_rmw(ocelot, > + (stream->sfid_valid ? ANA_TABLES_STREAMDATA_SFID_VALID : 0) | > + ANA_TABLES_STREAMDATA_SFID(stream->sfid), > + ANA_TABLES_STREAMDATA_SFID_VALID | > + ANA_TABLES_STREAMDATA_SFID_M, > + ANA_TABLES_STREAMDATA); > + > + reg = ocelot_read(ocelot, ANA_TABLES_STREAMDATA); > + reg &= (ANA_TABLES_STREAMDATA_SFID_VALID | ANA_TABLES_STREAMDATA_SSID_VALID); > + entry.type = (reg ? ENTRYTYPE_LOCKED : ENTRYTYPE_NORMAL); So if the STREAMDATA entry for this SFID was valid, you mark the MAC table entry as static, otherwise you mark it as ageable? Why? > + ether_addr_copy(entry.mac, stream->dmac); > + entry.vid = stream->vid; > + > + reg = ocelot_read(ocelot, ANA_TABLES_MACACCESS); > + dst_idx = (reg & ANA_TABLES_MACACCESS_DEST_IDX_M) >> 3; > + > + ocelot_mact_write(ocelot, dst_idx, &entry, row, col); > + > + return 0; > +} > + > +static int vsc9959_stream_table_add(struct ocelot *ocelot, > + struct list_head *stream_list, > + struct felix_stream *stream, > + struct netlink_ext_ack *extack) > +{ > + struct felix_stream *stream_entry; > + int ret; > + > + stream_entry = kzalloc(sizeof(*stream_entry), GFP_KERNEL); > + if (!stream_entry) > + return -ENOMEM; > + > + memcpy(stream_entry, stream, sizeof(*stream_entry)); > + > + ret = vsc9959_mact_stream_set(ocelot, stream, extack); > + if (ret) { > + kfree(stream_entry); > + return ret; > + } > + > + list_add_tail(&stream_entry->list, stream_list); > + > + return 0; > +} > + > +static bool vsc9959_stream_table_lookup(struct list_head *stream_list, > + struct felix_stream *stream) > +{ > + struct felix_stream *tmp; > + > + list_for_each_entry(tmp, stream_list, list) > + if (ether_addr_equal(tmp->dmac, stream->dmac) && > + tmp->vid == stream->vid) > + return true; > + > + return false; > +} > + > +static struct felix_stream * > +vsc9959_stream_table_get(struct list_head *stream_list, unsigned long id) > +{ > + struct felix_stream *tmp; > + > + list_for_each_entry(tmp, stream_list, list) > + if (tmp->id == id) > + return tmp; > + > + return NULL; > +} > + > +static void vsc9959_stream_table_del(struct ocelot *ocelot, > + struct felix_stream *stream) > +{ > + vsc9959_mact_stream_set(ocelot, stream, NULL); > + > + list_del(&stream->list); > + kfree(stream); > +} > + > +static u32 vsc9959_sfi_access_status(struct ocelot *ocelot) > +{ > + return ocelot_read(ocelot, ANA_TABLES_SFIDACCESS); > +} > + > +static int vsc9959_psfp_sfi_set(struct ocelot *ocelot, > + struct felix_stream_filter *sfi) > +{ > + u32 val; > + > + if (sfi->index > VSC9959_PSFP_SFID_MAX) > + return -EINVAL; > + > + if (!sfi->enable) { > + ocelot_write(ocelot, ANA_TABLES_SFIDTIDX_SFID_INDEX(sfi->index), > + ANA_TABLES_SFIDTIDX); > + > + val = ANA_TABLES_SFIDACCESS_SFID_TBL_CMD(SFIDACCESS_CMD_WRITE); > + ocelot_write(ocelot, val, ANA_TABLES_SFIDACCESS); > + > + return readx_poll_timeout(vsc9959_sfi_access_status, ocelot, val, > + (!ANA_TABLES_SFIDACCESS_SFID_TBL_CMD(val)), > + 10, 100000); > + } > + > + if (sfi->sgid > VSC9959_PSFP_GATE_ID_MAX || > + sfi->fmid > VSC9959_PSFP_POLICER_MAX) > + return -EINVAL; > + > + ocelot_write(ocelot, > + (sfi->sg_valid ? ANA_TABLES_SFIDTIDX_SGID_VALID : 0) | > + ANA_TABLES_SFIDTIDX_SGID(sfi->sgid) | > + (sfi->fm_valid ? ANA_TABLES_SFIDTIDX_POL_ENA : 0) | > + ANA_TABLES_SFIDTIDX_POL_IDX(sfi->fmid) | > + ANA_TABLES_SFIDTIDX_SFID_INDEX(sfi->index), > + ANA_TABLES_SFIDTIDX); > + > + ocelot_write(ocelot, > + (sfi->prio_valid ? ANA_TABLES_SFIDACCESS_IGR_PRIO_MATCH_ENA : 0) | > + ANA_TABLES_SFIDACCESS_IGR_PRIO(sfi->prio) | > + ANA_TABLES_SFIDACCESS_MAX_SDU_LEN(sfi->maxsdu) | > + ANA_TABLES_SFIDACCESS_SFID_TBL_CMD(SFIDACCESS_CMD_WRITE), > + ANA_TABLES_SFIDACCESS); > + > + return readx_poll_timeout(vsc9959_sfi_access_status, ocelot, val, > + (!ANA_TABLES_SFIDACCESS_SFID_TBL_CMD(val)), > + 10, 100000); > +} > + > +static int vsc9959_psfp_sfi_table_add(struct ocelot *ocelot, > + struct felix_stream_filter *sfi) > +{ > + struct felix_stream_filter *sfi_entry, *tmp; > + struct list_head *pos, *q, *last; > + struct ocelot_psfp_list *psfp; > + u32 insert = 0; > + int ret; > + > + psfp = &ocelot->psfp; > + last = &psfp->sfi_list; > + > + list_for_each_safe(pos, q, &psfp->sfi_list) { > + tmp = list_entry(pos, struct felix_stream_filter, list); > + if (sfi->sg_valid == tmp->sg_valid && > + sfi->fm_valid == tmp->fm_valid && > + tmp->sgid == sfi->sgid && > + tmp->fmid == sfi->fmid) { > + sfi->index = tmp->index; > + refcount_inc(&tmp->refcount); > + return 0; > + } > + /* Make sure that the index is increasing in order. */ > + if (tmp->index == insert) { > + last = pos; > + insert++; > + } > + } > + sfi->index = insert; > + > + sfi_entry = kzalloc(sizeof(*sfi_entry), GFP_KERNEL); > + if (!sfi_entry) > + return -ENOMEM; > + > + memcpy(sfi_entry, sfi, sizeof(*sfi_entry)); > + refcount_set(&sfi_entry->refcount, 1); > + > + ret = vsc9959_psfp_sfi_set(ocelot, sfi_entry); > + if (ret) { > + kfree(sfi_entry); > + return ret; > + } > + > + list_add(&sfi_entry->list, last); > + > + return 0; > +} > + > +static struct felix_stream_filter * > +vsc9959_psfp_sfi_table_get(struct list_head *sfi_list, u32 index) This function needs to be introduced in the patch where it is used, otherwise: https://patchwork.hopto.org/static/nipa/539543/12466413/build_32bit/stderr > +{ > + struct felix_stream_filter *tmp; > + > + list_for_each_entry(tmp, sfi_list, list) > + if (tmp->index == index) > + return tmp; > + > + return NULL; > +}
Hi Vladimir, On Tue, Aug 31, 2021 at 15:55:26AM +0800, Vladimir Oltean wrote: > > +static int vsc9959_mact_stream_set(struct ocelot *ocelot, > > + struct felix_stream *stream, > > + struct netlink_ext_ack *extack) { > > + struct ocelot_mact_entry entry; > > + u32 row, col, reg, dst_idx; > > + int ret; > > + > > + /* Stream identification desn't support to add a stream with non > > + * existent MAC (The MAC entry has not been learned in MAC table). > > + */ > > Who will add the MAC entry to the MAC table in this design? The user? Yes, The MAC entry is always learned by hardware, user also can add it by using "bridge fdb add". > > > + ret = ocelot_mact_lookup(ocelot, stream->dmac, stream->vid, &row, > &col); > > + if (ret) { > > + if (extack) > > + NL_SET_ERR_MSG_MOD(extack, "Stream is not learned in MAC > table"); > > + return -EOPNOTSUPP; > > + } > > + > > + ocelot_rmw(ocelot, > > + (stream->sfid_valid ? ANA_TABLES_STREAMDATA_SFID_VALID : 0) > | > > + ANA_TABLES_STREAMDATA_SFID(stream->sfid), > > + ANA_TABLES_STREAMDATA_SFID_VALID | > > + ANA_TABLES_STREAMDATA_SFID_M, > > + ANA_TABLES_STREAMDATA); > > + > > + reg = ocelot_read(ocelot, ANA_TABLES_STREAMDATA); > > + reg &= (ANA_TABLES_STREAMDATA_SFID_VALID | > ANA_TABLES_STREAMDATA_SSID_VALID); > > + entry.type = (reg ? ENTRYTYPE_LOCKED : ENTRYTYPE_NORMAL); > > So if the STREAMDATA entry for this SFID was valid, you mark the MAC table > entry as static, otherwise you mark it as ageable? Why? If the MAC entry is learned by hardware, it's marked as ageable. When setting the PSFP filter on this stream, it needs to be set to static. For example, if the MAC table entry is not set to static, when link is down for a period of time, the MAC entry will be forgotten, and SFID information will be lost. After disable PSFP filter on the stream, there is no need to keep the MAC entry static, setting the type back to ageable can cope with network topology changes.
On Tue, Aug 31, 2021 at 08:41:36AM +0000, Xiaoliang Yang wrote: > Hi Vladimir, > > On Tue, Aug 31, 2021 at 15:55:26AM +0800, Vladimir Oltean wrote: > > > +static int vsc9959_mact_stream_set(struct ocelot *ocelot, > > > + struct felix_stream *stream, > > > + struct netlink_ext_ack *extack) { > > > + struct ocelot_mact_entry entry; > > > + u32 row, col, reg, dst_idx; > > > + int ret; > > > + > > > + /* Stream identification desn't support to add a stream with non > > > + * existent MAC (The MAC entry has not been learned in MAC table). > > > + */ > > > > Who will add the MAC entry to the MAC table in this design? The user? > > Yes, The MAC entry is always learned by hardware, user also can add it > by using "bridge fdb add". > > > So if the STREAMDATA entry for this SFID was valid, you mark the MAC table > > entry as static, otherwise you mark it as ageable? Why? > > If the MAC entry is learned by hardware, it's marked as ageable. When > setting the PSFP filter on this stream, it needs to be set to static. > For example, if the MAC table entry is not set to static, when link is > down for a period of time, the MAC entry will be forgotten, and SFID > information will be lost. > After disable PSFP filter on the stream, there is no need to keep the > MAC entry static, setting the type back to ageable can cope with > network topology changes. > So if the MAC table entry is ageable, will the TSN stream disappear from hardware even though it is still present in tc? I think in previous versions you were automatically installing a static MAC table entry when one was not present (either it was absent, or the entry was dynamically learned). Why did that change?
On Tue, Aug 31, 2021 at 16:46:13AM +0800, Vladimir Oltean wrote: > > > > +static int vsc9959_mact_stream_set(struct ocelot *ocelot, > > > > + struct felix_stream *stream, > > > > + struct netlink_ext_ack *extack) { > > > > + struct ocelot_mact_entry entry; > > > > + u32 row, col, reg, dst_idx; > > > > + int ret; > > > > + > > > > + /* Stream identification desn't support to add a stream with non > > > > + * existent MAC (The MAC entry has not been learned in MAC table). > > > > + */ > > > > > > Who will add the MAC entry to the MAC table in this design? The user? > > > > Yes, The MAC entry is always learned by hardware, user also can add it > > by using "bridge fdb add". > > > > > So if the STREAMDATA entry for this SFID was valid, you mark the MAC > > > table entry as static, otherwise you mark it as ageable? Why? > > > > If the MAC entry is learned by hardware, it's marked as ageable. When > > setting the PSFP filter on this stream, it needs to be set to static. > > For example, if the MAC table entry is not set to static, when link is > > down for a period of time, the MAC entry will be forgotten, and SFID > > information will be lost. > > After disable PSFP filter on the stream, there is no need to keep the > > MAC entry static, setting the type back to ageable can cope with > > network topology changes. > > > > So if the MAC table entry is ageable, will the TSN stream disappear from > hardware even though it is still present in tc? Yes, PSFP filter information on the stream will be lost in hardware. > > I think in previous versions you were automatically installing a static MAC table > entry when one was not present (either it was absent, or the entry was > dynamically learned). Why did that change? The PSFP gate and police action are set on ingress port, and " tc-filter" has no parameter to set the forward port for the filtered stream. And I also think that adding a FDB mac entry in tc-filter command is not good.
On Tue, Aug 31, 2021 at 12:07:54PM +0300, Vladimir Oltean wrote: > On Tue, Aug 31, 2021 at 08:59:57AM +0000, Xiaoliang Yang wrote: > > > I think in previous versions you were automatically installing a static MAC table > > > entry when one was not present (either it was absent, or the entry was > > > dynamically learned). Why did that change? > > > > The PSFP gate and police action are set on ingress port, and " > > tc-filter" has no parameter to set the forward port for the filtered > > stream. And I also think that adding a FDB mac entry in tc-filter > > command is not good. > > Fair enough, but if that's what you want, we'll need to think a lot > harder about how this needs to be modeled. > > Would you not have to protect against a 'bridge fdb del' erasing your > MAC table entry after you've set up the TSN stream on it? > > Right now, DSA does not even call the driver's .port_fdb_del method from > atomic context, just from deferred work context. So even if you wanted > to complain and say "cannot remove FDB entry until SFID stops pointing > at it", that would not be possible with today's code structure. > > And what would you do if the bridge wants to delete the FDB entry > irrevocably, like when the user wants to delete the bridge in its > entirety? You would still remain with filters in tc which are not backed > by any MAC table entry. > > Hmm.. > Either the TSN standards for PSFP and FRER are meant to be implemented > within the bridge driver itself, and not as part of tc, or the Microchip > implementation is very weird for wiring them into the bridge MAC table. > > Microchip people, any comments? In sja1105's implementation of PSFP (which is not standard-compliant as it is based on TTEthernet, but makes more sense anyway), the Virtual Links (SFIDs here) are not based on the FDB table, but match only on the given source port. They behave much more like ACL entries. The way I've modeled them in Linux was to force the user to offload multiple actions for the same tc-filter, both a redirect action and a police/gate action. https://www.kernel.org/doc/html/latest/networking/dsa/sja1105.html#time-based-ingress-policing I'm not saying this helps you, I'm just saying maybe the Microchip implementation is strange, but then again, I might be looking the wrong way at it.
On Tue, Aug 31, 2021 at 18:49:53PM +0300, Vladimir Oltean wrote > On Tue, Aug 31, 2021 at 09:59:11AM +0000, Xiaoliang Yang wrote: > > On Tue, Aug 31, 2021 at 17:18:00PM +0300, Vladimir Oltean wrote: > > > > > > I think in previous versions you were automatically installing > > > > > > a static MAC table entry when one was not present (either it > > > > > > was absent, or the entry was dynamically learned). Why did that > change? > > > > > > > > > > The PSFP gate and police action are set on ingress port, and " > > > > > tc-filter" has no parameter to set the forward port for the > > > > > filtered stream. And I also think that adding a FDB mac entry in > > > > > tc-filter command is not good. > > > > > > > > Fair enough, but if that's what you want, we'll need to think a > > > > lot harder about how this needs to be modeled. > > > > > > > > Would you not have to protect against a 'bridge fdb del' erasing > > > > your MAC table entry after you've set up the TSN stream on it? > > > > > > > > Right now, DSA does not even call the driver's .port_fdb_del > > > > method from atomic context, just from deferred work context. So > > > > even if you wanted to complain and say "cannot remove FDB entry > > > > until SFID stops pointing at it", that would not be possible with today's > code structure. > > > > > > > > And what would you do if the bridge wants to delete the FDB entry > > > > irrevocably, like when the user wants to delete the bridge in its > > > > entirety? You would still remain with filters in tc which are not > > > > backed by any MAC table entry. > > > > > > > > Hmm.. > > > > Either the TSN standards for PSFP and FRER are meant to be > > > > implemented within the bridge driver itself, and not as part of > > > > tc, or the Microchip implementation is very weird for wiring them > > > > into the bridge MAC > > > table. > > > > > > > > Microchip people, any comments? > > > > > > In sja1105's implementation of PSFP (which is not standard-compliant > > > as it is based on TTEthernet, but makes more sense anyway), the > > > Virtual Links (SFIDs > > > here) are not based on the FDB table, but match only on the given source > port. > > > They behave much more like ACL entries. > > > The way I've modeled them in Linux was to force the user to offload > > > multiple actions for the same tc-filter, both a redirect action and a > police/gate action. > > > https://www.kernel.org/doc/html/latest/networking/dsa/sja1105.html#t > > > ime-b > > > ased-ingress-policing > > > > > > I'm not saying this helps you, I'm just saying maybe the Microchip > > > implementation is strange, but then again, I might be looking the > > > wrong way at it. > > > > Yes, Using redirect action can give PSFP filter a forward port to add > > MAC table entry. But it also has the issue that when using "bridge fdb > > del" to delete the MAC entry will cause the tc-filter rule not > > working. > > We need to define the expected behavior. > > As far as the 802.1Q-2018 spec is concerned, there is no logical dependency > between the FDB lookup and the PSFP streams. But there seems to be no > explicit text that forbids it either, though. > > If you install a tc-redirect rule and offload it as a bridge FDB entry, it needs to > behave like a tc-redirect rule and not a bridge FDB entry. > So it only needs to match on the intended source port. I don't believe that is > possible. If it is, let's do that. > > To me, putting PSFP inside the bridge driver is completely outside of the > question. There is no evidence that it belongs there, and there are switch > implementations from other vendors where the FDB lookup process is > completely independent from the Qci stream identification process. > Anyway, this strategy of combining the two could only work for the NULL > stream identifiers in the first place (MAC DA + VLAN ID), not for the others (IP > Stream identification etc etc). > > So what remains, if nothing else is possible, is to require the user to manage > the bridge FDB entries, and make sure that the kernel side is sane, and does > not remain with broken data structures. That is going to be a PITA both for the > user and for the kernel side, because we are going to make the tc-flower filters > effectively depend upon the bridge state. > > Let's wait for some feedback from Microchip engineers, how they envisioned > this to be integrated with operating systems. Yes, Since the PSFP hardware module relies on the MAC table, this requires the User to manage bridge FDB entries to ensure PSFP works. Only set PSFP on the existing MAC table entries to ensure consistency. Any comments from Microchip engineers?
On Tue, 2021-08-31 at 10:49 +0000, Vladimir Oltean wrote: > On Tue, Aug 31, 2021 at 09:59:11AM +0000, Xiaoliang Yang wrote: > > On Tue, Aug 31, 2021 at 17:18:00PM +0300, Vladimir Oltean wrote: > > > > > > I think in previous versions you were automatically > > > > > > installing a > > > > > > static MAC table entry when one was not present (either it > > > > > > was > > > > > > absent, or the entry was dynamically learned). Why did that > > > > > > change? > > > > > > > > > > The PSFP gate and police action are set on ingress port, and > > > > > " > > > > > tc-filter" has no parameter to set the forward port for the > > > > > filtered > > > > > stream. And I also think that adding a FDB mac entry in tc- > > > > > filter > > > > > command is not good. > > > > > > > > Fair enough, but if that's what you want, we'll need to think a > > > > lot > > > > harder about how this needs to be modeled. > > > > > > > > Would you not have to protect against a 'bridge fdb del' > > > > erasing your > > > > MAC table entry after you've set up the TSN stream on it? > > > > > > > > Right now, DSA does not even call the driver's .port_fdb_del > > > > method > > > > from atomic context, just from deferred work context. So even > > > > if you > > > > wanted to complain and say "cannot remove FDB entry until SFID > > > > stops > > > > pointing at it", that would not be possible with today's code > > > > structure. > > > > > > > > And what would you do if the bridge wants to delete the FDB > > > > entry > > > > irrevocably, like when the user wants to delete the bridge in > > > > its > > > > entirety? You would still remain with filters in tc which are > > > > not > > > > backed by any MAC table entry. > > > > > > > > Hmm.. > > > > Either the TSN standards for PSFP and FRER are meant to be > > > > implemented > > > > within the bridge driver itself, and not as part of tc, or the > > > > Microchip implementation is very weird for wiring them into the > > > > bridge MAC > > > table. > > > > Microchip people, any comments? > > > > > > In sja1105's implementation of PSFP (which is not standard- > > > compliant as it is > > > based on TTEthernet, but makes more sense anyway), the Virtual > > > Links (SFIDs > > > here) are not based on the FDB table, but match only on the given > > > source port. > > > They behave much more like ACL entries. > > > The way I've modeled them in Linux was to force the user to > > > offload multiple > > > actions for the same tc-filter, both a redirect action and a > > > police/gate action. > > > https://www.kernel.org/doc/html/latest/networking/dsa/sja1105.html#time-b > > > ased-ingress-policing > > > > > > I'm not saying this helps you, I'm just saying maybe the > > > Microchip > > > implementation is strange, but then again, I might be looking the > > > wrong way > > > at it. > > > > Yes, Using redirect action can give PSFP filter a forward port to > > add > > MAC table entry. But it also has the issue that when using "bridge > > fdb > > del" to delete the MAC entry will cause the tc-filter rule not > > working. > > We need to define the expected behavior. > > As far as the 802.1Q-2018 spec is concerned, there is no logical > dependency between the FDB lookup and the PSFP streams. But there > seems > to be no explicit text that forbids it either, though. > > If you install a tc-redirect rule and offload it as a bridge FDB > entry, > it needs to behave like a tc-redirect rule and not a bridge FDB > entry. > So it only needs to match on the intended source port. I don't > believe > that is possible. If it is, let's do that. > > To me, putting PSFP inside the bridge driver is completely outside of > the question. There is no evidence that it belongs there, and there > are > switch implementations from other vendors where the FDB lookup > process > is completely independent from the Qci stream identification process. > Anyway, this strategy of combining the two could only work for the > NULL > stream identifiers in the first place (MAC DA + VLAN ID), not for the > others (IP Stream identification etc etc). > > So what remains, if nothing else is possible, is to require the user > to > manage the bridge FDB entries, and make sure that the kernel side is > sane, and does not remain with broken data structures. That is going > to > be a PITA both for the user and for the kernel side, because we are > going to make the tc-flower filters effectively depend upon the > bridge > state. > > Let's wait for some feedback from Microchip engineers, how they > envisioned this to be integrated with operating systems. We at Microchip agrees that it is a difficult task to map the PSFP implementation in Felix to the “tc flower” filter command, but please remember that Ocelot and its derivatives were designed long before the 802.1Qci standard was ratified and also before anyone has considered how to control it in Linux. We think that the best approach is to require the user to manage bridge FDB entries manually as suggested by Xiaoliang. Our newer PSFP designs uses the TCAM instead of the MAC table which maps a lot better to the “tc flower” filter command.
On Thu, Sep 09, 2021 at 01:33:57PM +0200, Joergen Andreasen wrote: > > > Yes, Using redirect action can give PSFP filter a forward port to > > > add > > > MAC table entry. But it also has the issue that when using "bridge > > > fdb > > > del" to delete the MAC entry will cause the tc-filter rule not > > > working. > > > > We need to define the expected behavior. > > > > As far as the 802.1Q-2018 spec is concerned, there is no logical > > dependency between the FDB lookup and the PSFP streams. But there > > seems > > to be no explicit text that forbids it either, though. > > > > If you install a tc-redirect rule and offload it as a bridge FDB > > entry, > > it needs to behave like a tc-redirect rule and not a bridge FDB > > entry. > > So it only needs to match on the intended source port. I don't > > believe > > that is possible. If it is, let's do that. > > > > To me, putting PSFP inside the bridge driver is completely outside of > > the question. There is no evidence that it belongs there, and there > > are > > switch implementations from other vendors where the FDB lookup > > process > > is completely independent from the Qci stream identification process. > > Anyway, this strategy of combining the two could only work for the > > NULL > > stream identifiers in the first place (MAC DA + VLAN ID), not for the > > others (IP Stream identification etc etc). > > > > So what remains, if nothing else is possible, is to require the user > > to > > manage the bridge FDB entries, and make sure that the kernel side is > > sane, and does not remain with broken data structures. That is going > > to > > be a PITA both for the user and for the kernel side, because we are > > going to make the tc-flower filters effectively depend upon the > > bridge > > state. > > > > Let's wait for some feedback from Microchip engineers, how they > > envisioned this to be integrated with operating systems. > > We at Microchip agrees that it is a difficult task to map the PSFP > implementation in Felix to the “tc flower” filter command, but please > remember that Ocelot and its derivatives were designed long before > the 802.1Qci standard was ratified and also before anyone has > considered how to control it in Linux. > > We think that the best approach is to require the user to manage > bridge FDB entries manually as suggested by Xiaoliang. > > Our newer PSFP designs uses the TCAM instead of the MAC table > which maps a lot better to the “tc flower” filter command. Well, that's even more unfortunate, because as explained by Ido here: https://lore.kernel.org/netdev/YSHzLKpixhCrrgJ0@shredder/ the return code from SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE event handlers is not propagated to br_switchdev_fdb_notify. So in effect, the device driver cannot stop the bridge from removing an FDB entry which would lead it to having a broken tc filter. Now, the ocelot switchdev driver uses the bridge bypass .ndo_fdb_add and .ndo_fdb_del, while DSA actually listens for switchdev events on the atomic call chain. So any solution needs to work with switchdev, not just with the bridge bypass ops.